Difference between revisions of "Preparing Code"
Line 11: | Line 11: | ||
===Before You Start=== | ===Before You Start=== | ||
Check [https://jira.secondlife.com/secure/Dashboard.jspa| JIRA Bug Tracker] to see if the issue has been submitted and its status. | Check [https://jira.secondlife.com/secure/Dashboard.jspa| JIRA Bug Tracker] to see if the issue has been submitted and its status. | ||
If you're going to write a patch, don't forget to sign and send in the [Project:Contribution_Agreement contribution agreement]. The sooner you do that, the sooner we can accept your patch. | |||
=== Coding Standard === | === Coding Standard === |
Revision as of 14:45, 11 June 2007
If you have a piece of source code (be it a fix, new feature, or optimization of existing code) submit the patches in a way that makes it easy for the Lindens to incorporate them into the source code.
Good Patch Practise
General
The best way to think about patches is that the people who are reviewing and applying your patches are the bottleneck in the system. If you can make it easier for us to understand and apply patches without lots of cleanup, we can spend more time reviewing and incorporating your work.
Before You Start
Check JIRA Bug Tracker to see if the issue has been submitted and its status.
If you're going to write a patch, don't forget to sign and send in the [Project:Contribution_Agreement contribution agreement]. The sooner you do that, the sooner we can accept your patch.
Coding Standard
Please read and follow the coding standard: Second Life coding standard
Comments
Please do not add issue numbers or your name to patches. We have to remove these by hand. Also, please be careful in how you add comments. Bracketing a patch with comments like "// PATCH START" and "// PATCH END" does not add any information, and adds to the manual cleanup we have to do.
If you want to make sure you get credited, then add yourself to doc/contributions.txt as part of the patch. That makes it easier for people to see the exact nature of the changes you've added.
In short, just add to the patch what is expected to appear in the final source.
One Patch for one Issue
Each patch should do exactly one thing. If you have three different bugs to fix, please submit three different patches. When you fold unrelated hunks into a single patch, we have to try to figure out which part of the patch applies to which bug, and split it out by hand, sometimes into several patches. Combo patches are particularly time consuming, and the manual splitting up makes it very easy to lose pieces of patches altogether.
Less is more
Smaller patches are easier to review and therefore more likely to be integrated quickly. The longer it takes for a patch to be incorporated, the higher the likelihood that the underlying code will change enough for the patch to fail. Please do not include formatting or other clean-up changes as that makes it harder to review and more likely to fail.
Creating a Patch
Patches should be submitted in Unidiff-Format. This format is simlar to simple diffs but with more detailed information and can be automatically integrated into the source. It is generated by calling diff -u <original file> <new file> (under Windows you will find the diff command as part of the CygWin project in C:\CYGWIN\BIN).
With the file names it is best to rename the original file to something like viewer_orig.cpp have the new file the same name as in the Second Life project and redirect the output to a file that ends with .patch
The patch command for a change viewer.cpp would then be something like diff -u --strip-trailing-cr linden/indra/newview/viewer_orig.cpp linden/indra/newview/viewer.cpp >viewer.patch and would create output file named viewer.patch like this:
--- linden/indra/newview/viewer_orig.cpp 2007-05-14 16:47:26.000000000 +0200 +++ linden/indra/newview/viewer.cpp 2007-05-22 08:49:50.484375000 +0200 @@ -6302,7 +6326,7 @@ llinfos << "Cleaning Up" << llendflush; - LLKeyframeMotion::flushKeyframeCache(); + LLKeyframeMotion::flushKeyframeCache(TRUE); // Must clean up texture references before viewer window is destroyed. LLHUDObject::cleanupHUDObjects(); @@ -6562,6 +6586,8 @@ delete gVFS; gVFS = NULL; + LLCurl::cleanup(); + // This will eventually be done in LLApp LLCommon::cleanupClass(); }
Submitting the Patch for Peer Review
Especially with larger or sensitive patches it is a good idea to first submit the patch for peer review on the mailing list. This way fellow developers can review the patch, uncover possible bugs, comment on undesired interactions or generally share ideas with you.
Just subscribe to the mailing list, then write an email to the list with your .patch file attached and with a description of what it does in the text or also as an attachment.
Submitting the Patch on JIRA
Finally, when you think the patch is ready for the Lindens, submit the patch on the JIRA Bug Tracker. Create a new issue or attach the patch to an existing entry as appropriate. Attach the .patch file itself as a file attachment and make sure that the check mark for Patch attached is set (you can do that when creating the issue or by choosing Edit for an existing one).
Also, offer a description of the patch (the why and what) as a separate file and/or as a description or comment with the issue.
Before you do that however, just have a look at existing patches through the Issues with patches attached filter on the JIRA main page to get a general idea how it is done.
Good luck!