zzHelix Community Patch Submission and Code Review

https://common.helixcommunity.org/2005/patch/index.html

 

https://common.helixcommunity.org/2005/patch/index.html

Helix Community Patch Submission and Code Review Process

The Helix community is keenly interested in reviewing changes you believe should be made to the Helix DNA code base -- those that fit the technical direction of the DNA and are of sufficient quality will get incorporated.

The process for patch submittal works similarly to many public collaboration projects, such as OpenOffice, Mozilla, Linux and others. All patch submittals must go through a code review process (CR). The process is stricter when a branch affected by the submittal is in code freeze or for STABLE and RELEASE branches. The process is also stricter when the submittal involves code shared with other groups. No submittals that are targeted for production code should be checked in without being reviewed. There are just a few steps to follow:

1. Before beginning the patch submittal process please be sure all feature coding has been completed and that the submittal has been fully tested. Testing of the submittal should include feature testing and leak testing. Server patch submittals should also include load testing as well as testing on Linux, Solaris and Win32 platforms.
2. Next step is to find the right project for your patch. The projects line up one-to-one with the top-level directory in your CVS working copy. So, if you made a change to a file underneath /common, you need to send your patch to the [e-mail] project alias. Please note however that *-restricted directories do not line up one-to-one with you the top-level directory and your patches will need to be submitted to the appropriate rarvcode* mailing lists.
3. Perform the 'cvs update' command in your source tree to make sure you are current with the branch that you have been working from. If you haven't recently synchronized your code with the branch you have been working from, you may have to resolve conflicts before submitting these patches.
4. For each main project (e.g. a directory like client, audio, video, etc), do a 'cvs diff -uw' and save the output into a file named projectname.diff. The resulting file will be a 'unified' diff, which is the format we use for code review in the Helix Community. A very large 'unified diff' has the potential of greatly slowing down the review process so it is advised to submit patches more incrementally, in smaller chunks, if at all possible.
5. Complete the Code Review Form. All patch submissions should have a Subject line reading: "CR: reason-or-project-or-description". For the "datatype", "common", and "filesystem" projects, or other projects used by both the Helix DNA Client and the Helix DNA Server, patches should include "CR-Client" or "CR-Server" as appropriate. The email should also include the attached diff file from the previous step. Please note that the diff files and new files should be sent as attachments and not inline. To ensure that your patches are accepted be sure you explicitly include one of the copyright statements in the form (the preferred method is to have signed and delivered a Joint Copyright Assignment to RealNetworks). Once all of the above requirements have been met send the email to the appropriate project mailing lists.
6. Watch your inbox for discussion about your patch as developers inside and outside of Real will have a chance to review the changes for bugs, memory leaks and any other normal kinds of programming errors. If the submitted patch is considered shared code, all affected project owners must be given the opportunity to review it. Individual project owners can be found on the associated project home page. If no feedback is given in 24hrs, the following will happen:
   1. Submitter needs to re-send the CR request with the message titled "CR-Resend:" and mark the message as high priority.
   2. The project owners of the area CR'ed either will do the CR themselves or assign the CR to someone on the architecture team. All reviewers are required to respond within 24hrs. If the CR can't be completed in 24hrs, the reviewer is required to respond to the submitter with an estimated time.
7. After your patches have been reviewed by the project owner you will receive feedback as to whether the submittal is acceptable or changes need to be made. Reviewers of your code will review these modifications very closely and determine what if any risks are associated with your submittal. Reviewers will be considering:
   1. Is the benefit of this submittal greater than the risk?
   2. If it does not support all platforms, why? And is there a less platform-specific way to accomplish the same thing?
   3. Has it been fully tested?
   4. Is there a simpler/safer way to accomplish the same thing?
   5. Does the change require an intellectual property review?
8. If no changes are necessary and your submittal has been approved you will need to complete the Change Notification Form and send it to the project mailing list, qa-contact (if any assigned) and the program manager (if any assigned). The subject line of this email should read: CN-Client: <title> or CN-Server: <title>. Please note that approval from a branch owner AND a project owner is required when the two are not the same. Branch owners can be identified in the Client Branch Manifest.
9. Once you have completed this step and if you have CVS commit access then you are free to check in the code. If you do not have CVS commit access then the project development lead that reviewed your patch will submit it.
   Note: Please be sure to copy the contents of the Change Notification Form and paste it into the CVS Commit log. This helps other developers track and understand the reasons for the change from the CVS archives.
10. If you have developed off of a branch, the patches need to be merged into both that branch as well as to the HEAD in all cases.