... | ... | @@ -7,13 +7,13 @@ Some of the styles derived from BIND 9 that are often forgotten or misunderstood |
|
|
|
|
|
# Test-Driven Development
|
|
|
|
|
|
[Kea project](https://gitlab.isc.org/isc-projects/kea) proved beyond any doubt that [TDD](https://en.wikipedia.org/wiki/Test-driven_development) works and improves the code quality. We absolutely want to repeat the exercise in Stork. However, there are some lessons learned in Kea project. In general, unit-tests should be developed alongside the production code (before the code if possible) and included in the same MRs. Having said that, Kea project sometimes went a bit overboard with this and there were cases when people were wasting time implementing unit-tests for trivial getters and setters. That's not the goal here.
|
|
|
[Kea project](https://gitlab.isc.org/isc-projects/kea) proved beyond any doubt that [TDD](https://en.wikipedia.org/wiki/Test-driven_development) works and improves the code quality. We absolutely want to repeat the exercise in Stork. However, there are some lessons learned in Kea project. In general, unit-tests should be developed alongside the production code (before the code if possible) and included in the same MRs. Having said that, Kea project sometimes went a bit overboard with this and there were cases when people were wasting time implementing unit-tests for trivial getters and setters. That's not the goal here.
|
|
|
|
|
|
The intention is to make sure that the most complex or tricky parts of the code should be testable and we should have reasonable confidence in them being correct. There's another essential aspect here. If you feel that it's not possible to write unit-tests for your new code, perhaps your new code is structured badly? This is one of two major reasons why the tests cannot be written letter. The other is that there's never good time to come back and implement tests...
|
|
|
|
|
|
Finally, Stork is currently approved for 6 months. We don't want to have few features with super extensive tests. This could kill the project if the only thing we can show after 6 months is well written tests. On the other hand, we don't want to have tons of buggy features that don't work. We need to take a middle ground. Please use common sense. If in doubt, please ask @tomek for suggestions.
|
|
|
|
|
|
# Documentation
|
|
|
# Documentation
|
|
|
|
|
|
## Testing/Documentation addresses and prefixes
|
|
|
|
... | ... | @@ -23,7 +23,7 @@ Use 192.0.2.0/24 (see [RFC5737](https://tools.ietf.org/html/rfc5737) and 2001:db |
|
|
|
|
|
We sprinkle comments in code with keywords to indicate pending work.
|
|
|
|
|
|
In Stork, `TODO` is preferred. If there is a corresponding ticket, feel free to specify its number in the comment. Unless other wise specified, issue #1234 means a ticket in the Gilab, available at http://gitlab.isc.org/isc-projects/stork.
|
|
|
In Stork, `TODO` is preferred. If there is a corresponding ticket, feel free to specify its number in the comment. Unless other wise specified, issue #1234 means a ticket in the Gitlab, available at http://gitlab.isc.org/isc-projects/stork.
|
|
|
|
|
|
## Function/method comments
|
|
|
Every non-trivial (trivial means one or two liner, such as getter or setter) function and method MUST be documented. While it's appreciated, the documentation doesn't have to be very thorough. Well expressed sentence is often enough. Make sure the description is as useful as possible. "Returns URL string" is bad. "Returns agent's contact point URL as a string" is much better.
|
... | ... | @@ -31,17 +31,17 @@ Every non-trivial (trivial means one or two liner, such as getter or setter) fun |
|
|
## User's guide
|
|
|
The User's guide should be updated when a new feature is added. Again, the description doesn't have to be complex or extensive. A paragraph of text briefly announcing new feature or capability will help users (and other developers) a lot.
|
|
|
|
|
|
# Dead code
|
|
|
# Dead code
|
|
|
|
|
|
Dead code is bad; it suffers from code rot, and it looks unclean. There are some circumstances where there is a reason to keep a bit of unused code around for a while, but these should be the exception rather than the rule, and it should be very clear why it is there, and on what conditions and when it will be re-enabled or removed completely.
|
|
|
|
|
|
Any dead code (both files that are unused and blocks of commented-out code) should in principle be removed. If there is a very good reason to keep it around for a while, it must be accompanied by a comment explaining why it is still there, and when it will be removed or enabled again. This comment should point to a ticket so that we do not forget about it.
|
|
|
|
|
|
# Go Style
|
|
|
# Go Style
|
|
|
|
|
|
Used by backend server and agent. We use the syntax recommended by `go fmt` command. Please make sure your code adheres before you submit it for review.
|
|
|
|
|
|
# TypeScript Style
|
|
|
# TypeScript Style
|
|
|
|
|
|
Used by UI. Details TBD
|
|
|
|
... | ... | @@ -71,25 +71,25 @@ Use all all-lowercase characters for file names. Use dash as a separator (e.g. s |
|
|
|
|
|
## Ordering Imports in Go
|
|
|
|
|
|
We include first packages from standard library, then 3rd party packages and at the end our own project packages.
|
|
|
We include first packages from standard library, then 3rd party packages and at the end our own project packages.
|
|
|
|
|
|
## Line length
|
|
|
## Line length
|
|
|
|
|
|
The project kicked off in 2019. FullHD is ubiquitous with large 4K displays are getting common. In the general case, the code should have no more than 128 columns. This is let developers display two panels side by side. In some exceptional cases (such as URL), the code may be extended to 160 columns, but this should be rare occurrence.
|
|
|
|
|
|
## Tabs & Indentation
|
|
|
## Tabs & Indentation
|
|
|
|
|
|
Do not use hard tabs.
|
|
|
|
|
|
Indentation at each level is 4 spaces for C++, other languages should use what is "usual and expected."
|
|
|
|
|
|
## Naming
|
|
|
## Naming
|
|
|
|
|
|
Don't start things with underscores.
|
|
|
|
|
|
Class names are '''`LikeThis`''', methods are '''`likeThis()`''', variables are '''`like_this`''', and constants are '''`LIKE_THIS`'''. Data class members are '''`like_this_`'''.
|
|
|
|
|
|
## Comments
|
|
|
## Comments
|
|
|
|
|
|
Multiline comments can be written in C++ or C style (that is, with // or /* */ marks).
|
|
|
|
... | ... | @@ -119,7 +119,7 @@ class Foo { |
|
|
};
|
|
|
```
|
|
|
|
|
|
### Doxygen Comment Style
|
|
|
### Doxygen Comment Style
|
|
|
|
|
|
When writing a Doxygen special comment block there are several possible styles:
|
|
|
|
... | ... | @@ -160,44 +160,43 @@ class Good { |
|
|
};
|
|
|
```
|
|
|
|
|
|
### Explicit @brief for Doxygen
|
|
|
### Explicit @brief for Doxygen
|
|
|
|
|
|
If you don't use @brief as the first thing in your doxygen comment, then doxygen will turn the first paragraph into a @brief description anyway. However, we include it anyway so that everybody understands that this is the @brief description.
|
|
|
|
|
|
## Methods and Functions
|
|
|
## Methods and Functions
|
|
|
|
|
|
### Exception-safe getters and string-production methods
|
|
|
### Exception-safe getters and string-production methods
|
|
|
|
|
|
Unless there's a compelling reason to do so neither member value getter methods nor and string-production methods, such as toString(), should throw exceptions. Normally a class member is prevented from ever having an invalid value so there is arguably a getter never has a reason to throw, and string-production methods should always be safe to invoke once a class has been instantiated. Both types of methods are commonly used as log statement arguments where one should not have to worry about catching exceptions.
|
|
|
|
|
|
## Log Statement Safety
|
|
|
## Log Statement Safety
|
|
|
|
|
|
It is extremely important to examine all arguments passed into a log
|
|
|
statement to ensure they will produce safe values at runtime:
|
|
|
- Can the argument (or any part of it) be NULL? If so is this taken into account?
|
|
|
- If the argument invokes any fuctions, are they exception safe?
|
|
|
- If the argument invokes any functions, are they exception safe?
|
|
|
- If it involves indirection, does this always resolve into a usable value?
|
|
|
- If it raises an exception, is the exception caught? This includes double errors, i.e., log statements in an exception handler.
|
|
|
|
|
|
Log statements are less than helpful if they cause the program to segfault or throw.
|
|
|
Log statements are less than helpful if they cause the program to segfault or throw.
|
|
|
|
|
|
# User Interface (UI) Guidelines
|
|
|
# User Interface (UI) Guidelines
|
|
|
|
|
|
- The minimum resolution needed to reasonably use Stork UI is 720p (1280x720).
|
|
|
- Other aspects are TBD
|
|
|
|
|
|
# Imported Code
|
|
|
# Imported Code
|
|
|
|
|
|
If you import code from another project, try to continue the style of the imported project if changes need to be made. This is for two reasons, one is to make merging future versions easier. The other is the encouragement of submitting changes upstream.
|
|
|
|
|
|
# Guidelines Adopted by Other Projects
|
|
|
# Guidelines Adopted by Other Projects
|
|
|
|
|
|
Other projects have their own coding guidelines. Here're some
|
|
|
examples of such guidelines. These are reference purposes only;
|
|
|
unless explicitly stated we also adopt some part of other guidelines,
|
|
|
they are not part of the Stork's coding guidelines.
|
|
|
|
|
|
* Google: http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml
|
|
|
* Mozilla: https://developer.mozilla.org/en/Mozilla_Coding_Style_Guide
|
|
|
* XORP: http://cvsweb.xorp.org/cgi-bin/cvsweb.cgi/xorp/devnotes/coding-style.txt?rev=1.7;content-type=text%2Fplain
|
|
|
|
|
|
* Google: https://google.github.io/styleguide/cppguide.html
|
|
|
* Mozilla: https://firefox-source-docs.mozilla.org/code-quality/coding-style/index.html
|
|
|
* XORP: https://fossies.org/linux/xorp/devnotes/coding-style.txt |