Menu

Development Process

Anders Widell Mathi Naickan

Note: this page will soon be updated to reflect the new workflow when using [GIT] and the new branching strategy.

Writing a ticket

Before you start to develop a new feature or fix a bug in OpenSAF, make sure there is a ticket for the task in the OpenSAF ticket system. If there is no ticket yet, you must [write a ticket] yourself. Check with the affected service's maintainer to make sure he/she thinks the new feature is reasonable before you start to work on it, to avoid spending time implementing a feature that may risk being rejected from inclusion in OpenSAF. If the service doesn't have a maintainer, or if it is a system-level feature, you should check with the TLC members. You can find the list of maintainers and TLC members in the file AUTHORS at the top of the source code repository.

Starting to work on a ticket

When you 'start' working on a ticket, the below guidelines are applicable:

(a) The ticket should be accepted - i.e. change the ticket status to accepted and set yourself as the owner of the ticket. This way, other people can see what you are working on and we reduce the risk that two people are working on the same task without knowing about it.

(b) Set the Milestone.

  • When working on a Defect, set the milestone field to the oldest release where the code will be included. This will help in planning the release and tracking what is included in the different OpenSAF releases. The default value in the milestone field is configured(in the tool) to contain the oldest maintenance release in the pipeline.
    For example:-
    Suppose the two maintenance releases in pipeline are 4.5.x and 4.6.y. Between these two, 4.5.x is the oldest.
    If the fix for a defect has to be pushed into all of 4.5.x and 4.6.y and default branches, then you should set the milestone to 4.5.x. But, if the fix has to be pushed only to the 4.6.y and default branches, then you should set the milestone to 4.6.y.

  • When working on an Enhancement ticket, then the fix should only be included in the latest release that is currently under development, i.e. the 'default' branch. For example, if the next enhancement release is 4.z, then the Milestone shall be set to 4.z.FC where FC stands for "feature complete".
    Also, Defects can be uncovered while testing features that are pushed during an ongoing enhancements release, i.e. GA tag is not yet applied on that enhancements branch.
    For such defects the following guidelines are applicable:-

  • For defects uncovered when testing the 4.z.FC taggged code, the milestone field shall be set to the next milestone which is most likely the 4.z.RC1.
  • Similairly, for defects uncovered when testing the 4.z.RC1 code, the milestone field shall be set to 4.z.RC2 or 4.z.0 accordingly as applicable.

(c) Optionally, you may also update the Version field of the ticket to the first version of OpenSAF where the bug was introduced.

Notes
- We maintain the three latest releases of OpenSAF including the release currently under development(i.e. default branch).
- All enhancement tickets that are not being currently worked upon should have the Milestone field set to 'future'.

Setting up the development environment

Before you start the actual coding, make sure you have configured your [development tools] correctly. In particular, you need to configure [Mercurial] and your text editor (e.g. [Emacs]). Also read through our [Coding Rules] and make sure to follow them when writing the code.

Selecting a branch in the source code repository

Normally, development is done on the default branch in the source code repository. Enhancement tickets (i.e. new features) are only pushed to the default branch, but defect tickets (bug fixes) shall normally be pushed on all active branches where the bug exists. Bug fixes can normally also be developed on the default branch unless there is a big difference between the branches.

Splitting the code into multiple changesets

A single ticket can (and sometimes should) be pushed as several separate Mercurial changesets. One reason for splitting the code into several changesets is if the code affects several OpenSAF components (as listed in the ticket system in the field Component), or even several parts of the same component (also listed in the ticket system in the field Part). Another reason to split the code into separate changesets is to avoid mixing whitespace changes with functional changes. You should never mix whitespace changes and functional changes since this makes the functional changes difficult to review. A third reason for splitting the code into separate changesets is if the ticket is implemented as several logically separate parts (which is a good idea if this is possible). However, one thing to keep in mind is to never break the repository, i.e. it should always be possible to successfully build and start OpenSAF at any changeset, even if you pick a changeset in the middle of a set of patches that are pushed together as part of the same ticket. The reason for this is that someone might want to use the Mercurial command bisect to find the changeset where a regression was introduced. So for example if you have replaced some common library function, you could split this change into three changesets: first one changeset that adds the new functions, then a second changeset that modifies the code to use the new functions instead of the old ones, and then finally a third changeset that removes the old functions.

Writing good commit messages

Make sure each changeset you create has a proper commit message that follows the guidelines in the file 00-README, which is located in the tools/devel/review directory in the source code repository. In the same directory, there is also a file containing a commit message template that you can use as a staring point when writing a new commit message. The 00-README file explains how to set up Mercurial to load this template into the text editor automatically when prompting for a commit message.

Testing the code

If possible, you should add test cases for the new feature that you have implemented. Test cases are stored in the tests directory at the top of the OpenSAF source code repository. Note that the tests here are currently limited to SAF API tests that must be possible to run on a single node in the cluster. We are planning to add other types of tests, for example unit tests and cluster-wide tests.

Sending out the code for review

When the code is ready and tested, it shall be sent out for review. This is done using the script tools/devel/review/submit-review.sh which is found in the source code repository. In the same directory, there is a file called 00-README with instructions for how to use the script. Reviewer should normally be the maintainer(s) of the component that is affected, and in case of system-level changes you should also add TLC members as reviewers. See the AUTHORS file for a list of maintainers and TLC members. Only maintainers have push access to the repository, so if you are not a maintainer you should use the line "Pull request to: " to specify who you think should push the code to the repository once the review has been approved.

Receiving review comments

When the ticket has been sent out for review, change the status of the ticket to review. Then wait for people to give comments on the code. Please be patient and allow the reviewers to look at the code when they have time to do so - they might be busy with other tasks and not have time to review your code at the moment. It is not unusual that you will have to wait a week before receiving comments, for a large and/or difficult change even longer. Anyone can of course give comments, not just the people you listed as reviewer(s). But it is the reviewers that will have to reply with an "Ack" to confirm that they approve the changes, or possibly with a "Nack" to reject the changes. Once the reviewer(s) have Acked the patches, they can be pushed to the Mercurial repository.

Pushing the code

After the code has been pushed, change the ticket status to fixed and post a comment on the ticket where you list the changesets that were pushed (i.e. the printout of the hg outgoing command before you pushed). Sometimes a large ticket may be implemented in several iterations and in this case you change the status back to accepted instead of fixed and continue working on the next set of patches.


Related

Wiki: Coding Rules
Wiki: Emacs
Wiki: GIT
Wiki: Home
Wiki: Mercurial
Wiki: development tools
Wiki: write a ticket