How to request and perform a code review
The code review is a best practice that generally helps for early bugs detection and thus for increasing significantly the quality level of a software.
There are two different scenarious for code reviews depending on if the code is belonging to the EDNA kernel or not:
Code reviews for the EDNA kernel
Since all EDNA projects use the same kernel, it is of highest importance that it's stable. Therefore a developer should always send a code review to other developers for comments before committing his/her code related to a bug fix or a new feature of the kernel.
To request a code review, a developer has to create a ticket on the edna bugzilla server (see above). Setting a code review dependency on a bug is very convenient for follow-up purposes:
- The 'component' field should be set to 'codeReview'
- The 'summary' should begin with 'Review:
- The 'description' should be as explicit as possible regarding the purpose of the new code, some tricky parts of the codes, etc...
- The 'assignedTo' should be filled with the name of an identified reviewer and cc could be field as well for other comments request.
Once the ticket has been created, a pull request can be done, see below:
How to perform a code review for the EDNA kernel
- Fork the EDNA git repository on github (create an account if necessary): http://help.github.com/fork-a-repo
- Commit the changes into the forked repository
- Make a pull request: http://help.github.com/send-pull-requests
- One of the kernel developers will evaluate the pull request and eventually merge into the official EDNA repository
If the code review is not concerning the EDNA kernel, the code can be committed prior to the code review provided that the following requirements are met:
- If the development concerns changes to existing code, the developer should have the agreement of the principal author before committing the code.
- If the development is a completely new application, or a new plugin in an existing application, the developer should have received the agreement from the EDNA project manager or the EDNA project coordinator prior to committing the new code.
- If the development contains a new source code file for an existing plugin, it is preferred to have the agreement of the person(s) responsible for the other source code files in the same plugin.
The developer should have opened a bug in the EDNA bugzilla server (see page http://www.edna-site.org/cgi-bin/bugzilla/enter_bug.cgi?product=EDNA) prior to the commit. This bug could either be an existing bug, for example opened after a bug report, or a new code review bug if no existing bug exists.
It's very important that the bug number appears in the commit message, which should also clearly explain the purpose of the commit.
Once the commit has been made, the developer should report the new SVN revision number to the bug corresponding to the commit. He/she should then assign this bug to a code reviewer. The developer should also send a message to the edna-code-review mailing list with the following information:
- The purpose of the code review
- The chosen code reviewer
- The bug number of the code review
How to perform the code review
If the person assigned to the code review accepts to do the review, he/she adds questions and remarks to the bugzilla bug. The developer might then make further commits to the code already committed, remembering to always mention the bug number in the commit message. Once the developer and reviewer agrees that the committed code is ok the bug can be closed.
If the person assigned to the code review cannot make the review, he/she must assign either assign the code review to someone else or bounce it back to the project manager.
More than one person is of course welcome to give comments on the code.