Coding Guidelines

[cgrigis]

To help ensuring all the code related to MedCo is written in a "homogeneous" way, it would be good to write a Coding Guidelines document centralizing all these aspects, such as:

• How logging is performed (package used, message format, levels, etc.).
• How errors are handled (format, etc.).
• The accepted length of lines of code (e.g. 80 characters)
• The commenting approach (function parameters, complex code, verbosity, etc.)
• Naming (variables, packages, etc.). See e.g. Go package names.
• Policy on acceptable length for a function
• Common antipatterns (no magic values, etc., no code that is commented out, no code pertaining to testing in production code, etc.)
• Versioning policy following semver.

The formatting of the code itself is fortunately well handled by the standard Go tools (gofmt, golint, go vet).

Some issues currently identified:

• Most modules use https://github.com/dedis/onet, except medco-connector which uses https://github.com/sirupsen/logrus.
• Most modules use errors, except medco-unlynx which uses xerrors.
• medco-deplopyment has a tag v0.2.1-1 which does not follow semver (v0.2.1-1 < v0.2.1).
• Some files have with very long lines, e.g. here, here, here, here, here, here, etc.
• medco-connector has a full test commented out.
• Some files contains cod with magic values, e.g. timeout value, timeout value, channel size.
• Some files contain very long functions, e.g. here, here, here (could be split into smaller parts and factor code with loadV1()), etc.
• Some variable names are not very clear, e.g. typeQ (and not described in comments).
• Some files contains fairly complex code, and would benefit from additional comments, e.g. here.
• The package names loadergenomic and loaderi2b2 do not match the paths where they reside (i.e. genomic and i2b2). Furthermore, the main function from these packages could be named Load(), which would result in shorter yet clearer i2b2.Load() and genomic.Load() calls in the client.
• Errors in medco-loader do not appear to be handled uniformly: sometimes with log.Error() + return, sometimes with log.Fatal().