This is an internal documentation. There is a good chance you’re looking for something else. See Disclaimer.
Contributing Guidelines¶
General¶
No code is committed that is not 100% understood by the author.
A change is always reviewed by another developer. A change is locally tested before it is pushed to Gerrit.
If multiple independent changes are done they should be separately committed.
There is an editorconfig to maintain a consistent code layout. It defines the file encoding, linebreaks, indentation and trailing whitespaces.
The file encoding is always UTF-8.
Code and comments should be always in American English.
Do not comment out unused code. The version control system tracks the changes.
Commit Message¶
On the first line is the Gerrit subject. There is a limit of 72 characters. The message is written in the present (E.g. change and not changed or changes)
If more details are helpful add a message body. Between the subject and message body must be an empty line.
Use the Refs or Closes command to link your commit to a Jira task.
The Gerrit hook generates a change-id which is part of the commit message. Do not modify this id.
Here a full example:
add new monitoring module
- add logic to track requests
- add visualization of requests over time
- add custom JSON exporter
Refs: TOCDEV-1234
Change-Id: I35106653d15655c1b7d988ff0062da9fc9e41893
Code Reviews¶
- Every changeset has to be reviewed by another developer (via gerrit). No reviewer is required if:
Only text resources are adjusted
An error in a form is fixed (e.g. order of fields is changed)
A layout issue in a report is fixed
An auto-merge conflict is fixed and no logic is manually adjusted (e.g. only imports are fixed or
ImmutableList
is replaced byList
)
If non-customer code is changed, this part of the change must be reviewed by a DEV
When doing code reviews be respectful and remember also to praise good code.
Be clear and add suggestions.
Use conventional comments for being precise and clear in your intent.
The author has the responsibility to merge a pull / merge request.
Conventional comments¶
Conventional Comments documentation
<label> [decorations]: <subject>
[discussion]
label
- This is a single label that signifies what kind of comment is being left.subject
- This is the main message of the comment.decorations
(optional) - These are extra decorating labels for the comment. They are surrounded by parentheses and comma-separated.discussion
(optional) - This contains supporting statements, context, reasoning, and anything else to help communicate the “why” and “next steps” for resolving the comment.
We use the following set of labels:
Label
Description
Blocking / Non Blocking
praise:
Highlights something positive.
Non Blocking
suggestion:
Suggestions propose improvements. Be explicit and clear on what is being suggested and why!
both (use decorations)
question:
Questions can be used to ask the author for clarification or investigation of a specific topic.
both (use decorations)
todo:
TODO’s are small, trivial, but necessary changes.
Blocking
typo:
Similar to
todo:
where main issue is a mispelling.both (use decorations)
nitpick:
Nitpick is a trivial preference-based suggestion.
Non Blocking
We use the following set of decorations:
Decorations
Description
(non-blocking)
The comment does not block the code to be approved.
(blocking)
The comment requests a mandatory change to the code to get an approval.
(if-minor)
The comment can be done when the changes are small and do not take a lot of work or risk.
Examples:
typo: mispelled variable name, `isVaild` => `isValid`
suggestion (blocking): Reuse existing helper functions
We have already good tested helper functions for this date calcuation (see `modules/date`)
suggestion (non-blocking,if-minor): Can we refactor this class as well?
nice2validate¶
Several configuration errors are caught by validators that are run by our CD tool. See .ci/validators/bin/
in the repository
for the validators.
These validators can also be executed as a pre-push
hook. A gradle task setupPrePush
should run automatically when importing
the project which copies the hook to the correct location. If it didn’t, you can just run it yourself.
This requires some dependencies to be installed on your system. If they are not installed, a message gets logged and no pre-push hook will be setup. At point of writing, the dependencies are:
python3
pip
yamllint
parallel
Install these through apt install
, brew install
or whatever other packager manager or method you use.
If you want to push without these validators being run, you can use the --no-verify
argument when pushing from the command line,
or disable the Run git hooks
in IntelliJ Idea.
Java¶
General¶
Always include curly brackets, even for one-line if statements or for loops.
One statement per line. Avoid too long line (make a line break after ~ 120 characters).
Avoid the if ternary operator.
Each interface has a corresponding unit test named InterfaceName Test.java
Strings are concatenated with
String.format(...)
and not"String1" + "String2"
.Create lists and sets which are always empty with
List.of()
andSet.of()
. Else use the Guava library to create lists and sets (E.g.ImmutableList.of(1, 2)
).Each mock in a test must be verified.
A class has the following structure (sorting of the different class elements):
Variables:
Constants (
static final
)Injected services
Other variables such as mocks in tests
Constructors
Methods: First public, then protected, and then private methods are defined
Naming¶
Interfaces have no I-prefix. If an interface has only one implementation this class has the suffix Impl (E.g. Interface:
PizzaFactory
and ImplementationPizzaFactoryImpl
).Methods and field names are declared in camel case (E.g.
getCmsPage()
).Constants are declared in uppercase separated by underscores (E.g.
private static final int CACHE_SIZE = 100;
).Class members are always private and getter/setters are used for interaction.
Parameters and Return Values¶
The
@Nullable
annotation from theorg.jetbrains.annotations
package is used. But parameters and return values are not annotated with@NotNull
.An alternative to a nullable return value is an optional from Java (not Guava).
Parameters are validated if the input is in the expected range. Else an
IllegalArgumentException
is thrown.
Exception Handling¶
Catch blocks are never empty. At least log the error.
Use the logger and not
System.{out, err, ...}
orException#printStrackTrace()
.Before an exception is thrown the reason should be logged.
A method should throw only checked exceptions which are relevant for the caller. Less relevant exceptions, which you do not want to handle yourself, should be wrapped in a general exception.
Documentation¶
Use JavaDoc for documentation
Interfaces and methods which are not private have JavaDoc especially if they are in the
api
/spi
package and available in other modules (Exceptions are getters & setters and standard Java beans). The documentation describes the purpose of the class and points out special cases etc.Information about the author (
@author
) and the creation time (@since
) is not relevant in JavaDoc - for this we have a VCS.JavaDoc for methods (except private ones) contains at least (if useful)
@throws
,@returns
,@param
.Use the markdown syntax for formatting and not HTML.
Use Swagger annotations to document REST endpoints
Use a context specific documentation format such as the documentation attribute of the
@Batchjob
annotation
ExtJS¶
General¶
Always include curly brackets, even for one-line if statements or for loops.
One statement per line. Avoid too long line (make a line break after ~ 120 characters).
Do comparison operators always with the type-safe operator
===
and!==
.Do not use
Console.{log, error, ...}
.Usually do no custom error handling, because in case of an error a standard message is displayed.
If something did not work on the server, then throw an exception. Only then the failure method can be called on the client (and the user can be informed accordingly). If further information is needed in case of an error, then implement an own exception, which implements the
ch.tocco.nice2.netui.api.dwr.InformationException
. This allows to access its information in the failure method.In case of an error, keep the latest version if possible (e.g. do not close an open window).
Naming¶
Methods and field names are declared in a camel case (E.g.
getCmsPage()
).If a scope variable is needed, it is named
me
( →var me = this;
and not that, self, …). In general, you should not use scope variables and execute a function directly in the correct scope (E.g.myFunc.call(scope);
).If an action was successful the user should get a success message.
If an error occurs the user should always get an error message.
XML¶
Use hyphen syntax for element and attribute names, i.e., all lowercase letters; do not use camel case for elements (E. g.
set-status-date
).If a publicform is customer specific changed it cannot be partially replaced. It requires:
<form xmlns="http://nice2.tocco.ch/schema/formModel.xsd" data="My_entity" replace="true">
Non trivial entities, relations and fields are documented with the
<documentation>
element.