html_src/code-submission.html
author Tom Henderson <tomh@tomh.org>
Tue Jun 02 06:43:05 2009 -0700 (2009-06-02)
changeset 186 e83a8abbc8bb
parent 183 30b6889fbe44
child 217 cf02d0b73a17
permissions -rw-r--r--
incorporate Mathieu's code submission page into main web site
     1 <h1>How to submit new code to ns-3</h1>
     2 
     3 <h2>Introduction</h2>
     4 
     5 
     6 <p>This document summarizes the steps to take to prepare code for submission
     7 to ns-3 as a new feature, such as a new feature of an existing simulation
     8 model, or a new model altogether.  If you are interested in submitting a 
     9 bugfix to an existing ns-3 feature, or new documentation, see the companion 
    10 sections <a href="contributing.html#bugs">How to submit a bug report</a> or 
    11 <a href="contributing.html#documentation">How to submit documentation</a> to ns-3.</p>
    12 
    13 <p>We actively encourage submission of new features to ns-3.  
    14 Independent submissions are essential for open source projects, and
    15 you will also be credited as an author of future versions of ns-3.  
    16 However, please keep in mind that there is already a large burden on the 
    17 ns-3 maintainers to manage the flow of incoming contributions and maintain 
    18 new and existing code.  The goal of this document is thus to outline 
    19 how you can help to minimize this burden and thus minimize the average 
    20 time-to-merge of your code. Making sure that each code submission fulfills 
    21 as many items as possible in the following checklist is the best way to 
    22 ensure quick merging of your code. 
    23 </p>
    24 
    25 <p>
    26 In brief, we can summarize the guidelines as follows, and expand on them
    27 in detail below:
    28 <ul>
    29 <li>follow the ns-3 coding style
    30 <li>be licensed appropriately
    31 <li>write associated documentation, tests, examples
    32 <li>include a short description with each submission
    33 <li>minimize the size of each submission
    34 <li>changes should be coherent!
    35 <li>split your code into multi-part submissions
    36 <li>before sending a submission: ask yourself <i>if I were a reviewer, would I understand the submission?</i>
    37 <li>pick a submission tool:
    38   <ul>
    39     <li>a simple patch
    40     <li>a binary bundle
    41     <li>a hosted clone
    42     <li>patch emails
    43   </ul>
    44 <li>consider rewriting your development history
    45 </ul>
    46 <p>
    47 If you do not have the time to follow through the process to include your
    48 code in the main tree, please see <a href="#out-of-tree">below</a> about 
    49 contributing ns-3 code that is not maintained in the main tree.
    50 </p>
    51 
    52 <h2>Licensing and coding style</h2>
    53 
    54 <h3>Licensing, copyright, and authorship</h3>
    55 
    56 <p>
    57 All code submitted must conform to the project licensing framework, which
    58 is GNU GPLv2 compatibility.  All new source files must contain a 
    59 license statement.  All modified source files must either conform to
    60 the license of the original file or (in rare cases) may add a 
    61 license to the original license.  
    62 
    63 <p>Note that it is incumbent upon the submitter to make sure that the licensing
    64 attribution is correct and that the code is suitable for ns-3 inclusion.
    65 <i>Do not</i> include code (even snippets) from sources that have incompatible
    66 licenses.
    67 
    68 <p>If you are writing a new file or making large modifications (more than 20 or 30 lines)
    69 to an existing file, include a copyright statement in the header, such as: 
    70 <tt>Copyright 2009 John Doe</tt>. Do not use the phrase <tt>All rights reserved.</tt>.
    71 Optionally, you may also add a comment to help us track you down in case of problems in
    72 the code such as <tt>Author:  john_doe@example.edu</tt>. However, do not create a chain of authors by
    73 appending to this list in the header such as <tt>Modified by Jane Doe 02/04/09</tt>;
    74 we use Mercurial change history to log who is changing what file.
    75 
    76 <p>You may append any contributors to the <tt>AUTHORS</tt> file as you see fit.
    77 
    78 <h3>Coding style</h3>
    79 
    80 <p>
    81 Whatever code you submit, make sure that you follow the ns-3 <a href="http://www.nsnam.org/codingstyle.html">coding style</a>.
    82 
    83 <p>Some of the rules it describes might seem arbitrary to you and you
    84 might disagree with these rules because the resulting code layout
    85 looks ugly to you.  Even so, we ask you to consider carefully the 
    86 purpose of a coding style: the purpose is to ensure that the overall
    87 codebase is consistently indented and laid out to make it easier for a 
    88 trained eye to deal with large amounts of code. 
    89 
    90 <p>So, whether you disagree with our layout rules, or you are unsure
    91 about whether a specific piece of code matches the rules, focus
    92 on making sure that the coding style of your code matches the coding 
    93 style of any surrounding ns-3 code.
    94 
    95 <p>Of course, minor style errors and uncertainties can be easily 
    96 corrected during reviews, but code submissions with a widescale 
    97 ignorance of the ns-3 coding style will likely be bounced back until 
    98 this step is taken.
    99 
   100 <h3>Whitespace changes</h3>
   101 
   102 <p>Do not ever submit code which contains whitespace/tab changes mixed
   103 with other non-whitespace changes. Generally, it's a good idea to avoid
   104 submitting whitespace changes at all, but, if you really must, submit
   105 a single change consisting only of whitespace changes.
   106 
   107 <h2>Completing your code submission</h2>
   108 
   109 <p>Often we have observed that contributors will provide nice new classes
   110 implementing a new model, but will fail to include supporting test
   111 code, example scripts, and documentation.  Therefore, we ask people
   112 submitting the code (who are in the best position to do this type of
   113 work) to provide documentation, test code, and example scripts.
   114 
   115 <p>Note that you may want to go through multiple phases of code reviews,
   116 and all of this supporting material may not be needed at the first stage (e.g.
   117 when you want some feedback on public API header declarations only, before
   118 delving into implementation).  However, when it times come to merge your
   119 code, you should be prepared to provide these things, as fits your
   120 contribution (maintainers will provide some guidance here).
   121 
   122 <h3>Including examples</h3>
   123 
   124 <p>For many submissions, it will be important to include at least one example
   125 that exercises the new code, so the reviewer understands how it is intended
   126 to be used.  For final submission, please consider to add as many examples
   127 as you can that will help new users of your code.  The <tt>samples/</tt> and <tt>examples/</tt>
   128 directories are places for these files.
   129 
   130 <h3>Testing</h3>
   131 
   132 <p>Every new code should include automated tests.  These should, where 
   133 appropriate, be unit tests of model correctness, and may be validation
   134 tests of stochastic behavior.  The test code should try to cover as 
   135 much model code as possible, and should be checked as much as possible
   136 by regression tests.  The project is still refining the testing framework
   137 and will add more detail to this section later, but suffice it to say
   138 for now that new features should come with test code to match; otherwise
   139 it risks falling into disrepair over time.  
   140 
   141 <p>
   142 TODO:  Add a reference (perhaps a manual chapter), with some examples and 
   143 samples, of how to write good ns-3 tests.
   144 
   145 <h3>Documentation</h3>
   146 
   147 <p>If you add a new features, or make changes to existing features, you
   148 need to update existing or write new documentation and example code.
   149 Consider the following checklist:
   150 <ul>
   151   <li>doxygen should be added to the header files, and should be checked
   152     for doxygen errors
   153   <li>incompatible API changes must be documented in <tt>CHANGES.html</tt>
   154   <li>new features should be described in <tt>RELEASE_NOTES</tt>
   155   <li>new API or changes to existing API must update the inline doxygen
   156     documentation in header files
   157   <li>consider updating or adding a new section to the tutorial in <tt>doc/tutorial/</tt>
   158   <li>consider updating or adding a new section to the manual in <tt>doc/manual/</tt>
   159   <li>update the <tt>AUTHORS</tt> list as appropriate
   160 </ul>
   161 
   162 <h2>Preparing your code for review</h2>
   163 
   164 <h3>Description</h3>
   165 
   166 <p>For each code submission, include a description of what your code is
   167 doing, and why. Ideally, you should be able to provide a summary
   168 description in a 5-line paragraph with a 1-line (15 word) subject. If
   169 you fix a bug filed in our <a href="http://www.nsnam.org/bugzilla">bugzilla database</a>, 
   170 the subject should include first the bug number, and then, the bug title.
   171 For example: <tt>bug 558: qos-tag.h is gone missing from wscript</tt>
   172 
   173 <p>Bonus points go to submitters who provide a description of their 
   174 testing strategy.
   175 
   176 <h3>Minimize the size of each submission</h3>
   177 
   178 <p>Each submission should be as small as possible. Ideally, each submission
   179 should deal with one issue only. If your description of your submission 
   180 includes words such as <i>and</i>, it is a big red warning sign that you
   181 should think about splitting your submission in two separate smaller 
   182 submissions, if the changes are not intertwined.
   183 
   184 <h3>Coherent changes</h3>
   185 
   186 <p>Each submission should be a coherent whole: if you need to edit ten
   187 files to get a feature to work, then, the submission should contain all
   188 the changes for these ten files. Of course, if you can split the feature 
   189 in sub-features, then, you should do it to decrease the size of the 
   190 submission as per the previous item.
   191 
   192 <p>For example, if you have made changes to optimized a module and to fix
   193 a bug in another module, make sure you separate these two sets of changes
   194 in two separate submissions.
   195 
   196 <h3>Multi-part submissions</h3>
   197 
   198 <p>If you are working on a large new feature or a large refactoring, and 
   199 because you will attempt to minimize the size of your submissions, you
   200 will have to split your large work in multiple separate submissions.
   201 
   202 <p>Ideally, these submissions will be started by a detailed explanation
   203 of the overall plan such that code reviewers can review each submission
   204 separately but within a large context. This kind of work typically is
   205 split in multiple dependent steps where each step depends on the previous
   206 one. If this is the case, make it very clear in your initial explanation.
   207 If you can, minimize dependencies between each step such that reviewers
   208 can merge each step separately without having to consider the impact of
   209 merging one submission on other submissions.
   210 
   211 <h3>Before sending a submission</h3>
   212 
   213 <p>When you send a submission for review and merging in ns-3, before
   214 you hit the 'Send' button of your email client, ask yourself one 
   215 last time: <i>If I were a reviewer, and I had to review that submission,
   216 what would I do ?</i>. Specifically, make sure that you have provided 
   217 enough context to allow someone else to understand what you did and why.
   218 Keep in mind that your reviewer does not have access to a readable
   219 dump of your brain: he has access only to your code, and your emails.
   220 
   221 <h3>Submission tools</h3>
   222 
   223 <p>
   224 There are many ways to submit a piece of code. Some of them are not
   225 appropriate:
   226 <ul>
   227   <li>do not send us a modified version of each file you have changed
   228   <li>do not send us a .zip or .tar.gz file by email with a copy of
   229 every modified file
   230   <li>do not send us a patch against an unspecified version of ns-3
   231 </ul>
   232 
   233 <p>Others are more appropriate:
   234 <ol>
   235   <li>a simple patch
   236   <li>patch reviews at <a href="http://codereview.appspot.com">codereview</a>
   237   <li>a binary bundle
   238   <li>a hosted clone
   239   <li>patchbombs
   240 </ol>
   241 
   242 <ol>
   243 <li><b>a simple patch</b>
   244 
   245 <p>Patches can be attached to a bug report or sent to our developer mailing-lists.
   246 
   247 <p>The UNIX <tt>diff</tt> tool is the most common way of producing a patch: a 
   248 patch is a text-based representation of the difference between
   249 two text files or two directories with text files in them. If you have
   250 two files, <tt>original.cc</tt>, and, <tt>modified.cc</tt>, you can generate a patch
   251 with the command <tt>diff -u original.cc modified.cc</tt>. If you wish to
   252 produce a patch between two directories, use the command 
   253 <tt>diff -uprN original modified</tt>.
   254 
   255 <p>Make sure you specify to the reviewer where the original files came 
   256 from and make sure that the resulting patch file does not contain 
   257 unexpected content by performing a final inspection of the patch 
   258 file yourself.
   259 
   260 <p>Patches such as this are sufficient for simple bug fixes or very simple
   261 small features.
   262 
   263 <li><b>patch reviews at <a href="http://codereview.appspot.com">codereview</a></b>
   264 
   265 <p>The best way to submit a larger patch for consideration is to request 
   266 a patch review:
   267 
   268 <ol>
   269 <li>download <a href="http://codereview.appspot.com/static/upload.py">upload.py</a>
   270 <li>record the changes you want to request a review for in a mercurial 
   271 repository: <tt>hg commit ...</tt>
   272 <li>within the mercurial repository, run upload.py. Make sure you specify 
   273 <tt>ns-3-reviews@googlegroups.com</tt> as a CC.
   274 <li>announce your review request on ns-developers 
   275 </ol>
   276 
   277 <p>When you send a tree without a detailed summary of your changes, it 
   278 would help if you could send a list of the changesets you want to merge. 
   279 To generate it, first merge with ns-3-dev and then, from your modified 
   280 directory, run <tt>hg outgoing -p http://code.nsnam.org/ns-3-dev</tt>
   281 
   282 <p>If you already pulled changes from ns-3-dev into your private repository 
   283 after you started doing your private modifications, there is an issue to 
   284 be considered. upload.py does not let you specify a range of revisions, 
   285 nor a set of changesets. So if you just run <tt>upload.py</tt> in your private 
   286 repository, the changesets pulled from ns-3-dev will also be published 
   287 on codereview, which is, of course, not desirable.
   288 
   289 <p>A possible workaround is to pull your changes into a temporary 
   290 repository which is an up-to-date clone of ns-3-dev. The following code 
   291 should do the trick. This code assumes that your private repository is in 
   292 path DEV_BRANCH_WITH_NEW_FEATURE, and that it is in sync with ns-3-dev.
   293 
   294 <pre>
   295 hg clone http://code.nsnam.org/ns-3-dev ns-3-tmp
   296 cd ns-3-tmp
   297 export REVNO=`hg tip -q | sed 's/:.*$//'`
   298 hg pull DEV_BRANCH_WITH_NEW_FEATURE
   299 hg merge
   300 hg commit -m "merged new feature"
   301 upload.py --rev=$REVNO --cc=ns-3-reviews@googlegroups.com 
   302 </pre>
   303 
   304 <li><b>Binary bundles</b>
   305 
   306 
   307 <p>ns-3 uses <a href="http://www.selenic.com/mercurial/">mercurial</a> as its 
   308 version control tool: a quick introduction to using it can be found on our 
   309 <a href="http://www.nsnam.org/mercurial.html.">homepage</a>.
   310 Further extensive documentation and manuals are available from the mercurial
   311 <a href="http://www.selenic.com/mercurial/">homepage</a>.
   312 
   313 <p>If you get used to use mercurial to record your daily/hourly work using
   314 mercurial in a local clone of the ns-3 main repository, and if you want to 
   315 share that work with other people to get initial feedback about your work,
   316 your implementation approach, and start a high-level discussion about it,
   317 you can easily package all your local work history as a single binary bundle
   318 file with the <tt>hg bundle</tt> command and send this file over email. 
   319 
   320 <li><b>Hosted clones</b>
   321 
   322 
   323 <p>If your bundles grow large in size, it can become tricky to send them over
   324 email: hosting a clone of your mercurial repository is a simple way to work
   325 around this. If you have a host with a static ip address and/or a hostname,
   326 you can run <tt>hg serve</tt> within your repository in this host and send instead
   327 to your co-workers a url to that repository: if I run <tt>hg serve -d -p 8080</tt> 
   328 on my local ns-3-dev clone, I could send the url <tt>http://diese.inria.fr:8080/</tt>
   329 to my co-workers to allow them to pull all my development history from there.
   330 
   331 <p>If you don't have a static ip address or hostname, or are located behind
   332 a NAT or a firewall, you have to use a third-party hosting provider for your
   333 code. A small list of hosting providers is available 
   334 <a href="http://www.selenic.com/mercurial/wiki/index.cgi/MercurialHosting">there</a>
   335 
   336 
   337 <p>We can also provide hosting to ns-3 developers on <a href="http://code.nsnam.org/">code.nsnam.org</a>
   338 should none of the above alternative work for you.
   339 
   340 <li><b>Patchbombs</b>
   341 
   342 <p>Patchbombs are a nice way to perform a request for a detailed
   343 review. Make sure you enable the patchbomb extension in your ~/.hgrc:
   344 <pre>
   345 [extensions]
   346 hgext.patchbomb = 
   347 </pre>
   348 Make sure you configure your patchbomb extension correctly: 
   349 <tt>man 5 hgrc</tt> and look in the [email] and [smtp] sections
   350 
   351 Finally:
   352 <pre>
   353 hg email -i -t ns-developers@isi.edu http://code.nsnam.org/ns-3-dev
   354 </pre>
   355 
   356 <p>Beware that generating too many emails with this tool without doing
   357 proper <i>history rewriting</i> to produce clean easy-to-review patches will
   358 be considered rude.
   359 
   360 </ol>
   361 
   362 <h3>History rewriting</h3>
   363 
   364 <p>The idea behind history rewriting is to request a code submitter to 
   365 re-arrange their repository history prior to merging in the main
   366 ns-3 repository. 
   367 
   368 <p>For example, let's say that during development of a new feature, you
   369 decide to use your mercurial repository as a fancy version-enabled
   370 backup system: you do a lot of work, and regularly commit it to save it.
   371 When you are done implementing and testing that new feature, the resulting
   372 repository history as shown with <tt>hg log</tt> will look very verbose, and
   373 will most likely contain a lot of commit messages such as <i>fix bug</i>.
   374 Clearly, none of these commits are very helpful and there is little point
   375 in keeping them around: they make it painful to use the annotate, and bisect
   376 commands, and will make review of the final mercurial tree harder than it
   377 need to be.
   378 
   379 <p>A simple way to work around these problems is to ask each contributor to
   380 re-structure their commit history before submitting their tree for review:
   381 <ul>
   382   <li>pointless backup commits are deleted from the history
   383   <li>commits are re-organized in sub-feature commits
   384   <li>each commit is made buildable
   385   <li>etc.
   386 </ul>
   387 
   388 <ol>
   389 <li><b>Rewrite a new history from scratch</b>
   390 
   391 <p>Once you are done with a new feature, you can generate one final patch
   392 with the mercurial <tt>diff</tt> command and apply it to a clean repository
   393 as one single commit with a nice new commit message.
   394 
   395 <p>A variant on the above is to split the final commit in multiple smaller
   396 commits, each of which addresses one aspect of the final feature and
   397 make sure that each commit is still buildable.
   398 
   399 <li><b>Build clean history from the start</b>
   400 
   401 <p>Instead of using mercurial as a powerful backup system and trying to go back
   402 later, some users prefer to use tools such as 'mq' or 'pbranch' to split
   403 their work from the start in a set of separate entities and still record 
   404 somewhere their day-to-day development history.
   405 
   406 <p>More information on these tools:
   407 <ul>
   408 <li><a href="http://hgbook.red-bean.com/read/managing-change-with-mercurial-queues.html">Managing change with mercurial queues</a>
   409 <li><a href="http://hgbook.red-bean.com/read/advanced-uses-of-mercurial-queues.html">Advanced uses of mercurial queues</a>
   410 <li>The <a href="http://arrenbrecht.ch/mercurial/pbranch/">Pbranch</a> extension.
   411 </ul>
   412 
   413 </ol>
   414 
   415 <h2><a name="out-of-tree">Submitting out-of-tree code</a></h2>
   416 
   417 <p>Anyone who wants to provide access to code that has been developed
   418 to extend ns-3, but who chooses not to go through the above review process,
   419  may list its availability on our website.  Furthermore,
   420 we will provide storage on a web server if needed.  This type of code
   421 contribution will not be formally maintained by the project, although
   422 popular extensions might be adopted downstream.
   423 
   424 <p>We ask anyone who wishes to do this to provide at least this information
   425 on our wiki:
   426 <ul>
   427 <li>Authors,</li>
   428 <li>Name and description of the extension,</li>
   429 <li>How it is distributed (as a patch or full tarball),</li>
   430 <li>Location,</li>
   431 <li>Status (how it is maintained)</li>
   432 </ul>
   433 
   434 <p>The contribution will be stored on our wiki 
   435 <a href="wiki/index.php/Contributed_Code">here</a>.  
   436 If you need web server space for your extension, please
   437 contact one of the project maintainers.
   438