Difference between revisions of "Preparing Code"

Revision as of 14:16, 6 July 2017

If you have a piece of source code (be it a fix, new feature, or optimization of existing code) prepare your changes in a way that makes it easy for the Lindens to incorporate them into the source code.

Good Change Practice

It's easy for the people who review and apply changes to be the bottleneck in the system. If you can make it easier for Lindens to understand, test, and integrate your change, it is much better for everyone concerned.

Before You Start

Understand and use the Project Snowstorm Processes; making sure that your change is part of the plan is the most important thing you can do to get it accepted.

You must complete and sign the contribution agreement.

Coding Standard

Please read and follow the coding standard: Second Life coding standard. Several common problems can be avoided by running the checks provided by our Mercurial Tools.

Comments

Do not add issue numbers and your name to the code itself. While comments explaining code in a general working context are helpful and strongly encouraged, bracketing a change with comments like // PATCH START and // PATCH END does not add any information, and is likely to get your change returned for revision.

Do not comment out or use the preprocessor to disable code that you are replacing: just delete it. The source control system can find it again if we need it, and leaving dead code around makes things harder to read and much harder to search.

In short, make your change what is expected to appear in the final source.

Give Yourself Credit!

While names and JIRAs shouldn't appear in source, they do belong in doc/contributions.txt. Please don't be shy about adding the issue label and adding your name. If you already have one or more entries in contributions.txt, add new ones at the appropriate positions (sorting yours, if desired).

One Change For One Issue

Each submission should do exactly one thing. If you have three different bugs to fix, please submit three different change sets (unless they are very closely related and being treated as one item in the sprint). If you are unsure whether or not it is ok to combine changes into one submission, ask the Lindens.

Less Is More

Smaller change sets are easier to review and therefore more likely to be integrated quickly. This does not mean that you should omit comments or unit tests, however - adding to those is strongly encouraged.

When The Code Is Ready

See How To Submit A Viewer Change

@@ Line 1: / Line 1: @@
 {{OSWikiContribBox}}
+__NOTOC__
-If you have a piece of source code (be it a fix or a new feature) submit the patches in a way that makes it easy for the Lindens to incorporate them into the source code.
+If you have a piece of source code (be it a fix, new feature, or optimization of existing code) prepare your changes in a way that makes it easy for the Lindens to incorporate them into the source code.
-== Good Patch Practise ==
+== Good Change Practice ==
-=== General ===
+It's easy for the people who review and apply changes to be the bottleneck in the system. If you can make it easier for Lindens to understand, test, and integrate your change, it is much better for everyone concerned.
-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==
-=== Coding Standard ===
+* Understand and use the [[Project Snowstorm#Processes|Project Snowstorm Processes]]; making sure that your change is part of the plan is the most important thing you can do to get it accepted.
-Please read and follow the coding standard: [[Coding_standard|Second Life coding standard]]
+* You ''must'' complete and sign the [http://lecs-opensource.secondlife.com/SLVcontribution_agmt.pdf contribution agreement].
-=== Comments ===
+== Coding Standard ==
-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.
+Please read and follow the coding standard: [[Coding_standard|Second Life coding standard]].  Several common problems can be avoided by running the checks provided by our [[Mercurial Tools]].
-In short, just add to the patch what is expected to appear in the final source.
+== Comments ==
-=== One Patch for one Issue ===
+Do not add issue numbers and your name to the code itself.  While comments explaining code in a general working context are helpful and strongly encouraged, bracketing a change with comments like <code>// PATCH START</code> and <code>// PATCH END</code> does not add any information, and is likely to get your change returned for revision.
-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.
+Do not comment out or use the preprocessor to disable code that you are replacing: just delete it.  The source control system can find it again if we need it, and leaving dead code around makes things harder to read and much harder to search.
+In short, make your change what is expected to appear in the final source.
+== Give Yourself Credit! ==
-== Creating a Patch ==
+While names and JIRAs shouldn't appear in source, they do belong in <code>doc/contributions.txt</code>. Please don't be shy about adding the issue label and adding your name.  If you already have one or more entries in <code>contributions.txt</code>, add new ones at the appropriate positions (sorting yours, if desired).
-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).
+== One Change For One Issue ==
-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''
+Each submission should do exactly one thing. If you have three different bugs to fix, please submit three different change sets (unless they are very closely related and being treated as one item in the sprint).  If you are unsure whether or not it is ok to combine changes into one submission, ask the Lindens.
-The patch command for a change viewer.cpp would then be something like ''diff -u viewer_orig.cpp viewer.cpp >viewer.patch'' and would create output file named ''viewer.patch'' like this:
+== Less Is More ==
- --- linden\indra\newview\viewer_orig.cpp    2007-05-14 16:47:26.000000000 +0200
+Smaller change sets are easier to review and therefore more likely to be integrated quickly.  This does not mean that you should omit comments or unit tests, however - adding to those is strongly encouraged.
-  +++ 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();
- }
+==When The Code Is Ready==
-== Submitting the Patch for Peer Review ==
+See [[How To Submit A Viewer Change]]
-Especially with larger or sensitive patches it is a good idea to first submit the patch for peer review on the [[Developer_communication_tools|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 [https://jira.secondlife.com/secure/Dashboard.jspa|JIRA Bug Tracker].  Either create a new issue there or attach the patch to an existing entry.  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!