Skip to content

Developer guidelines

Jump to bottom
smoretti edited this page May 26, 2020 · 9 revisions

Table of content:

Git usage guidelines

Branching model

We follow the Git branching model as described here. In practice:

  • all development is made from the develop branch.
  • the master branch contains a production-ready code; so there is no merge in this branch in daily work.
  • when working on a new feature or issue, you should create a feature branch, and merge it back into the develop branch when the work is completed.
  • if a hotfix is needed, it should be branched off the master branch, and merged back into both the master and develop branches.
  • before making a new release, and updating the production-ready code in master, a release branch is branched off the develop branch, in order to make the necessary tests and modifications (e.g., changing the version number). This branch should then be merged into both the master and develop branches.

Bgee specific guidelines:

  • Note that we do not consider mandatory to push all feature branches to repository; but branches representing lots of work must be "saved" to the repository; such feature branches might then be removed from the repository after merge (git push <remote> --delete <branchName>)
  • Do not use a fast-forward merge, even if possible, when merging a feature branch containing several commits (git merge --no-ff <targetBranch>). This allows to see all modifications from the feature branch in one additional commit.
  • The commit message of the no-fastforward merge to the develop branch should contain the issue number it addresses (e.g., "see issue#xxx"). Do not close directly the issue from the commit message (our policy is to tag the issue as resolved, to be reviewed before closing).

For tips about how to use git, see our Git tips wiki page.

Task definitions and assignments

  • We use the issue tracker to define and assign tasks.
  • When starting a task defined in an issue, you should assign it to you, so that other team members know that you are working on it.
  • When the work on an issue is completed, label it as resolved, to be reviewed before closing. Do not close issues directly from Git commit messages.

Labels used so far. Usually, an issue is one of:

  • Enhancement and/or UI enhancement:
    • Enhancement: will add something interesting/useful from a user perspective.
    • UI enhancement: will add something interesting/useful from a user perspective, related to visualizations in the webapp.
  • Critical: essential to do for correct following development iterations.
  • bug: something broken, but it could still have a low priority if we don't really care...
  • Code: purely technical, issue only meant to improve code quality.
  • Suggestion: idea throwing, not yet decided whether to be implemented.
  • Discussion: well, as the label says...

Effort:

  • effort: low: 1-2 days of work for one person
  • effort: medium: about a week of work for one person
  • effort: high: a week or more of work for one person.

Priorities: priority: low, priority: medium, priority: high.

Code writing conventions

Notes and reminders in code

Use FIXME, TODO, and XXX.

  • FIXME is for things which are definitely broken, but where you want to not worry about it for the moment.
  • TODO is for useful features, optimizations or refactorings that might be worth doing in the future.
  • XXX is for things that require more thought and that are arguably broken.

See http://c2.com/cgi/wiki?FixmeComment for more details.

Patterns to use in everyday work

  • Expose only immutable objects
  • Use dependency injections
  • Favor composition over inheritance

Use assert extensively

  • See why: http://www.pgbovine.net/programming-with-asserts.htm. Or, also: "Java's assert statements are typically used for documenting (by means of assertions) under what circumstances methods can be invoked, and what their callers can expect to be true afterward. The assertions can optionally be checked at run time, resulting in an AssertionError exception if they don't hold."
  • It is valid to use assert for checking pre- and post-conditions of private methods, but I'd rather like to see a formal exception in that case...

Unit, integration, regression tests

  • All public methods, and eventually protected methods, should have a corresponding unit test, except basic getters/setters.
  • Unit tests should not rely on other dependencies (e.g., a database, or a HTTP resource). All needed dependencies should be mocked in unit tests.
  • When you discover a bug, you should implement a regression test, or modify a unit test to take into account the problematic scenario.
  • Integration tests rely on external dependencies, to verify that several components are working together as expected.

Methods lengths and dispatch

  • In most cases, a method should not do more than about 150 lines. Factorize your code in several methods if needed, with a distinct task for each of them.
  • Try avoiding getting a stack of method calls (a method calling a method calling a method calling a method...). It is more readable to have one main method, dispatching tasks to other methods, with as few interleaved levels of method calls as possible (i.e., an as small stack size as possible).

Javadoc

Providing javadoc is mandatory.

For documentation, see notably: http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html
Here are presented important outlines:

  • A doc comment must precede all class, field, constructor and method declarations.

  • The first sentence of a doc comment is very important. It should be a summary sentence, containing a concise but complete description of the relative comment. The Javadoc tool copies this first sentence to the appropriate member, class/interface or package summary. This makes it important to write crisp and informative initial sentences that can stand on their own. This sentence ends at the first period that is followed by a blank, tab, or line terminator, or at the first tag. See [http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#firstsentence first sentence] for more details.

  • It is mandatory to follow style guide: [http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#styleguide Style guide]. Notably: ** Use {@code} style for keywords and names. This includes java keywords, package names, class names, method names, field names, interface names, argument names, code examples. ** Use {@link} links economically ** Method descriptions begin with a verb phrase ** Add description beyond the API name: the ideal comment goes beyond words used e.g., in method names, and should always reward you with some bit of information that was not immediately obvious from the API name.

  • Required tags: ** an @param tag is "required" for every parameter, even when the description is obvious. ** the @return tag is required for every method that returns something other than void, even if it is redundant with the method description. ** @author is required at the class level. ** Do not document getters and setters

  • @param: the @param tag is followed by the name (not data type) of the parameter, followed by a description of the parameter. The first noun in the description is the data type of the parameter. (Articles like "a", "an", and "the" can precede the noun.). Additional spaces can be inserted between the name and description so that the descriptions line up in a block.
    Parameter names are lowercase by convention. The data type starts with a lowercase letter to indicate an object rather than a class. Do not bracket the name of the parameter after the @param tag with <code>...</code> since Javadoc 1.2 and later automatically do this. Examples: @param ch the character to be tested @param observer the image observer to be notified

  • Order of tags: ** @author (classes and interfaces only, required) ** @version (classes and interfaces only, required) ** @param (methods and constructors only) ** @return (methods only) ** @throws
    ** @see
    ** @since
    ** @serial
    ** @deprecated

Default constructors

If you have no constructor defined in a class, you must create an explicit 0-arg constructor, to be sure you didn't let it public by mistake.

From http://www.oracle.com/technetwork/articles/java/index-137868.html: "Good programming practice dictates that code should never make use of default constructors in public APIs: all constructors should be explicit. That is, all default constructors in public and protected classes should be turned into explicit constructor declarations with the appropriate access modifier. This explicit declaration also gives you a place to write documentation comments.

The reason this is good programming practice is that an explicit declaration helps prevents a class from inadvertently being made instantiable, as the design engineer has to actually make a decision about the constructor's access. We have had several cases where we did not want a public class to be instantiable, but the programmer overlooked the fact that its default constructor was public. If a class is inadvertently allowed to be instantiable in a released version of a product, upward compatibility dictates that the unintentional constructor be retained in future versions. Under these unfortunate circumstances, the constructor should be made explicit and deprecated (using @deprecated).

Note that when creating an explicit constructor, it must match precisely the declaration of the automatically generated constructor; even if the constructor should logically be protected, it must be made public to match the declaration of the automatically generated constructor, for compatibility. An appropriate doc comment should then be provided. Often, the comment should be something as simple as:

/**
 * Sole constructor.  (For invocation by subclass 
 * constructors, typically implicit.)
 */
protected AbstractMap() 
{
}

"

HTML and Javascript code conventions

  • Use of HTML5
  • No style information in the HTML code. Use of CSS3.
  • No javascript in the HTML code (except when using AngularJS, but it's not real javascript...). All the javascript code is totally externalized. That does not just mean that the js functions are externalized in separated files ; there is really no javascript at all in the HTML.

More picky stuff

Line Length

The maximum line length is about 100 (max 120) characters.

Indentation

When indenting your code, use four spaces. Never use tabs or mix tabs and spaces. Exception: When a line break occurs inside parentheses, align the wrapped line with the first character inside the parenthesis.

Spacing

  • Place spaces around all binary operators (=, +, -, *, etc.). Exception: Spaces around ='s are optional when passing parameters in a function call(not applicable to Java, but it is to javascript...).
  • Do not place a space before a comma, but always place one after a comma.
  • Place a space before left parenthesis, except in a function call.
  • Extra spacing (i.e., more than one space in a row) is okay if it improves alignment of equals signs.
  • Do not place spaces around code in parentheses or square brackets, but always place a space after a comma.

Curly Braces

  • An opening curly brace of a condition statement should never go on its own line; a closing curly brace should always go on its own line.
  • Always begin the body of a block on a new line.

Parting words

"Use common sense and BE CONSISTENT.

If you are editing code, take a few minutes to look at the code around you and determine its style. If others use spaces around their if clauses, you should, too. If their comments have little boxes of stars around them, make your comments have little boxes of stars around them, too.

The point of having style guidelines is to have a common vocabulary of coding so people can concentrate on what you are saying, rather than on how you are saying it."

Clone this wiki locally