User Tools

Site Tools


soft:guidelines

CONFINE project repository guidelines

This section is about how the CONFINE project repository is organized so everybody shares the same understanding on the layout.

The projects should share the same layout to ease the way things get done on different projects. There are two special projects (external) that will be explained later on.

Branch description

There are two branches for each project:

  • master: this is the stable branch.
    • There is one lead developer for each project.
    • No one should push changes to this branch (except for the lead developer).
    • It should be seen as a read-only branch for the rest of devs.
    • This branch will be used to make releases of our project and keep track of tags.
  • testing: this is the main integration branch (the jungle).
    • Use this branch to rebase your changes.
  • private developer branches:
    • It would be a good practice for each developer to make changes on a private branch before merging with testing.

All in all the layout is very similar to that described here.

Jenkins (http://testing.confine-project.eu at iMinds) will track the master and testing branches to fire a build every time there is push.

Git usage convention

The basic strategy we use is rebase-down, merge-up with no fast-forwarding (to keep branch history).

When starting some new development, rebase your private branch from testing to make sure you have all other developers’ changes.

$ git checkout testing
$ git pull origin testing
$ git whatchanged    ## to see what other folks have checked in
$ git checkout private-dev-branch
$ git rebase testing

Then you can make one or many commits. You may push private-dev-branch to GitHub, and pull it back down to other machines for testing. All this is private to you. When you have completed a set of changes, you will once again rebase private-dev-branch to testing, so that it is ready. Once your private branch is ready, you can merge it back to the testing branch, and announce that to the team. The cycle would go like:

$ ## Make some changes and commits to your private branch...
$ git checkout testing
$ git pull origin testing ## fetch possible changes from origin (remote repository)
$ git checkout private-dev-branch
$ git rebase testing    ## apply what others have changed or published
$ git checkout testing
$ git merge --no-ff --no-commit private-dev-branch
$ ## Review the merge if necessary, then commit.
$ git commit

Some differences between rebase and merge can be found here. Also a common problem of not understanding rebase is explained here.

Once in a while, when the testing branch is stable and ready to be released, the lead developer will merge with master and tag.

Common sense

  • Stick to one logical change per commit. You should not add multiple unrelated features in the same commit.

External forks

We also have two external projects in our CONFINE repository:

  • confine-openwrt and confine-packages: these are clones of the OpenWrt distribution (cross-compile environment + packages). They will be useful for us to not depend on the Subversion trunk where several commits are made every day.
    • There will be extra branches (trunk, attitude-adjustment, etc.) which will track the corresponding openwrt Subversion branch.
    • From time to time, the testing branch could be rebased with trunk so we can keep our distribution on the “bleeding edge” easily.
    • It would also be easier for us to make patches available upstream to the OpenWrt community.

HOWTO rebase testing with OpenWrt trunk:

  • Create a merge_trunk branch from testing.
  • Add the following lines on .git/config:
[remote "openwrt"]
    fetch = +refs/heads/*:refs/remotes/openwrt/*
    url = git://nbd.name/{openwrt,packages}.git (write "openwrt" OR "packages" depending on the repo you are working on)
  • Cross your fingers and execute the following:
git pull --rebase openwrt master
  • Try to fix any error or problem.

CONFINE project general coding guidelines

This section collects some general rules and tips to maintain a unique style on CONFINE project source code.

Use of magic constants

  • Magic constants should be avoided as they create a lot of maintenance problems.
  • Each constant used in the code should be declared in a “constants file” with a comment describing what it is used for.

General utility functions

  • It is quite common when writing a class that we need some auxiliar functions: converting strings from Unicode to ASCII, writing a string to a file, splitting URLs into their parts, etc. If we see that a function would be used from different classes it is good to put it on the utility class.
  • This is a static class that just serves as placeholder for these kind of functions.

Number of lines per method

  • It is recommended that method implementations do not exceed 300 lines. This is to improve readability.

Return codes

  • When using APIs that return error codes, always check if the result was an error and handle it. If the methods called throw exceptions, always catch any potential exceptions.

Method implementation layout

  • Each method should validate the parameters received and throw an error when the validators fail.
  • It is recommended that methods have only one “return” statement to make the code easier to follow.

Comments

  • Use meaningful variable names and add single line comments every few lines to explain what is the intention of the code. This generally improves maintainability and it makes it easier for you to remember after six months what code is doing.

Write code with testability in mind

  • Every class should have its own unit test file that you should develop as you write the code. Ideally, a test suite for the class should invoke at least each method in the class once with different parameters. Also, more complex test cases involving several methods should be developed.
  • The aim is that unit tests at least provide 60-70% of line coverage.
  • If your code depends on other classes try to make things extensible via interfaces, e.g. if your code relies on a file system class, make it easy to plug a test filesystem class that can inject different error conditions.
  • If you are writing a server application, make sure the whole server logic can be instantiated in isolation so different scenarios can be tested from a test environment.
  • Another good way of testing an implementation is by creating scenarios that simulate the real world behaviour of the code and also allow bechmarking portions of the code from outside the applciation. For example, if your are writing a server, create scenarios where a server is instantiated and several synthetic clients are created to simulate work load: each client chooses ar random a set of messages it issues before dying, the duration of the connection can be also chosen at random. Leaving this kind of tests running for hours can reveal some hard-to-find bugs.

Compiler settings

  • Try to compile at the highest level of warning (normally /W4 or -Wall), this helps identifying bugs early.

Memory and resource leaks

  • It is very important that the code that we write neves leaks resources. Server side applications can die very quickly if they leak a few bytes each time they handle a connection. For Linux there is an application called Valgrind that can help with this.

Resource deallocation

  • Two very common sources of bugs are resource leaks and double deletions.
  • The first occurs when the object allocates some resources and forgets to dispose them. To avoid this situation always perform visual inspections of code to see if there are code paths that can leak resources. The destructors should always deallocate any memory or resources.
  • If resrouces can be deallocated during the lifetime of the object, a cleanup method should be implemented and called from the destructor. To avoid deletion bugs (where a block of memory is freed twice), always reset to zero o NULL the variables disposed.

Use of assert

  • Assert is very useful to “assert” the state of the program at a particular point.
  • Assert can be used anywhere in the code but it is specially recommended for stating preconditions and postconditions, that is, the state of an object before an action executes and the expected state after the action is executed.

Check-ins on SCM

Before checking in code make sure that:

  • Your changes do not break any other projects. Remember that even test code must be fixed before checking in.
  • All test suites still pass after changes were made.
  • If it is possible, your code has been reviewed by another person. This takes a few minutes but it is useful to catch some bugs. Just describe to another person what your changes are and how things work now.
soft/guidelines.txt · Last modified: 2015/06/10 10:56 by santiago