Contribution rules

This page gathers all the rules and habits that have been taken by the contributors on the Sigmah code. It is useful to read it as an introduction when joining the project, to be acknowledgeed of how to fit your contribution in the same trend, in order to keep the development and the final perception of the user homogeneous.

Later on, if you perceive explicitly a new habit or wants to describe an implicit rule, you're welcome to update this page !

• Avoid to display in final release buttons or link to non-working or half-working part of the application: users will always get disappointed that something is not working yet.

We are several organizations and individuals working at the same time on the Sigmah codebase. To make this collaboration smooth for everyone requires the respect of several good housekeeping rules on our Git source code management.

If you want your contribution to be welcomed, make sure you publish your work in a friendly manner.

Write good commit messages, explain what you did :

• concise summary on first line (imperative tone is shorter), blank line, and more explanation below
• Each commit must be linked to one and only one Sigmah Mantis issue: on the last lines of the commit message, mention the issue you have worked on with one of text pieces of this exhaustive list: issue #1234, bug #1234, refs #1234, reports #1234 or if you want in the same time to set the issue as resolved you can use either Fixes #1234, fixes #1234 or resolves #1234
  • No reference to issue in source code comment The link to the issue needs only to be in the commit message. No need to add a comment in the modified part your source code with a reference to the issue because in the end it will be too complex to understand with many intricated issues applyed on the same area of the source code. Better to keep the link with issues in the Git log.
  • Side note: before a branch is merged into the official Sigmah repo, there can be issues registered in alternative issue trackers for that specific branch. But each commit made on that branch must respect the rule of a compulsory link with a unique issue in Sigmah Mantis. For better understanding of all commit message history in Sigmah codebase, reference to issues in alternative issue trackers should be made in the commit message through the title of the issue and not its id.
• read more (and more)
• Favor more small commits over less commits: commit frequently (even for one-line modification) for every changes that are standalone: each committed change should keep the build stable.
• History should be easy for a human to understand what you actually did, the diff should show what are the actual changes (for example, don't change indentation and change code: do a first commit for indentation changes and another-one for the actual change. Do separate commit for a refactoring that do not change the application behavior)

To prevent merge conflicts, the more you anticipate merging, the more your contribution is likely to be accepted.

• Publish your work, push often on your public repo (so others can see what you are doing, and may warn you of something)
• The earlier, the better. Submit pull-request as soon as a non-breaking changes has been done. We want your work of value, even if it's just a refactoring. The earlier it is merged, the more other contributor will base their work upon it, preventing conflicting modifications.
• Get early code review by creating pull-requests early and mark them as “WIP” (work-in-progress), pull-requests marked as WIP won't be merged, you can continue working on it and it won't be merged but you can get comments of other experienced Sigmah contributors (better know if you are on the wrong way fast)

• Check your commit integrity: the author must be the actual author of the code (pseudo are OK as long as you keep the same identity for all your contributions)
• Keep the graph simple: try pull --ff the official repo master in your fork master frequently to be up-to-date. Merge only when necessary. Rebase your local commit/branches as you want but do not rebase already pushed commits (or you might create duplicate commits)
• We aim at maintaining short-lived branches/forks:
  • Favor committing refactoring tasks first so it can be integrated early to prevent merge conflicts, even if the actual functionality code has not been started yet. Commit messages way starts with “Refactor …” to easily identify those
• One pull-request per functionality, one functionality per pull-request. Keep it simple and small.
• You may not change .gitignore and .gitattributes files, it will probably be rejected in a pull-request. Any temporary or generated files should go to the target directory.
• The main Sigmah repo may only be used by the URD team (and Code Lutin), you are encouraged to work on your very own public fork of the project (that's the github style). We encourage one fork/repo per contributing organization. Feel free to organize cooperation on your fork according to your organization preferences (feel free to create branches, plug your own CI server, etc…): we strongly encourage diversity of practices among teams working on Sigmah.

If you need to change the database schema to make your piece of code works, it's OK, but we want to keep a read-only history of all those modifications.

Your pull-request must come with a migration file, like src/main/resources/db/migration/V1234__adding_a_table_to_store_my_super_anything.sql. This file name contains the schema version, please just number it after the others files, and after the __, a brief description of your changes.

You can get more info in the official Flyway documentation.

• “Developers develop & translators translate”: In order to reduce the risks of conflict on the UIConstants, UIMessages and MailModels properties files, developers should avoid to edit the translated version of them. The French or Spanish developers can write the translation in their mothertongue of the new strings they create as part of the comment of the string in the original file.
  • In simpler words:
    • Developers only add new strings at the end of the ***.properties files (without suffix) in the part dedicated to the version on which they work, and if possible always with a comment which may include the translation in the mothertongue for Spanish or French developers
    • Developers should not edit the ***_$$.properties files (with suffix)
• “Give context to all my strings”: Translators love to have as much context as possible about what they have to translate, because it really helps their job. Thus, developers should try to add a comment (starting with the “#” character) on the line before each string they add. This comment should describe the string, and gives at best at least one screen in which it appears. It can also include the translation of the string in the mothertongue for French and Spanish developers.
• “I delete code, I check the constant strings”: Translators, as developers, don't like to work for no reason. So if a developer deletes some code containing an I18N string, and has a doubt whether this string is used anywhere else and could be deleted, he should check in ths code if this string is still used after his deletion. If it is no more used, it should be deleted.

Well, we won't discuss about how to produce good code, there are thousands of great literature about the topic.

Here are few coding guidelines that you should consider:

• Immutability is preferred over mutability (final keyword is on nearly every variables and method parameters), it made the code safer
• Fail fast (check arguments and throwing IllegalArgumentException and the beginning of a method is better than letting something unexpected happen)
• Future-proofing, if you do things like switch-case or else if structures, always implements a good default (always have a default: case or else with throwing an Exception or something). Think about a future developer who adds an element to an enumeration and this same developer forget to add a case in your code, did the code fail fast with an exception telling that this enumeration element is not implemented or do your code silently fall in another case leading to an expected behavior? Make the future developer (future you?) happy.
• We don't have a dummy rule like “a Javadoc on every method” but please, remove all boiler-plate and generated comment (“Here be Javadoc”…) and put comments where the code is not self-explanatory
• We like self-explanatory code with long explicit variables names, favor understandability over concision
• Consistency: adopting the not-in-your-habits code-style of the rest of the class/package is better than having a class coded with different styles from a method to another (also know as “when in Rome, do as the Romans do”)
• When throwing an exception:
  • always add a message, always use a different message for different throw (it will help when analyzing a stacktrace even if the code raising the exception has moved)
  • provide context that may be helpful to know the state of the application (ids, some variables values, configuration?)
  • if exception is thrown in a catch clause, always include the root exception that lead to you throwing another exception