code submission howto
authorMathieu Lacage <mathieu.lacage@sophia.inria.fr>
Wed May 06 14:55:46 2009 +0200 (9 months ago)
changeset 18330b6889fbe44
parent 181 a7b9e7b453bf
child 184 1c4287f1198e
code submission howto
html_src/code-submission.html
nsnam.css
     1.1 --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
     1.2 +++ b/html_src/code-submission.html	Wed May 06 14:55:46 2009 +0200
     1.3 @@ -0,0 +1,419 @@
     1.4 +<h1>How to submit new code to ns-3</h1>
     1.5 +
     1.6 +<h2>Introduction</h2>
     1.7 +
     1.8 +
     1.9 +<p>This document summarizes the steps to take to prepare code for submission
    1.10 +to ns-3 as a new feature, such as a new feature of an existing simulation
    1.11 +model, or a new model altogether.  If you are interested in submitting a 
    1.12 +bugfix to an existing ns-3 feature, or new documentation, see the companion 
    1.13 +page <a href="bug-submission.html">How to submit a bug report</a> or 
    1.14 +<a href="doc-submission.html">How to submit documentation</a> to ns-3.</p>
    1.15 +
    1.16 +<p>First, we actively encourage submission of new features to ns-3.  
    1.17 +Independent submissions are essential for open source projects, and
    1.18 +you will also be credited as an author of future versions of ns-3.  
    1.19 +However, please keep in mind that there is already a large burden on the 
    1.20 +ns-3 maintainers to manage the flow of incoming contributions and maintain 
    1.21 +new and existing code.  The goal of this document is thus to outline 
    1.22 +how you can help to minimize this burden and thus minimize the average 
    1.23 +time-to-merge of your code. Making sure that each code submission fulfills 
    1.24 +as many items as possible in the following checklist is the best way to 
    1.25 +ensure quick merging of your code. 
    1.26 +</p>
    1.27 +
    1.28 +<p>
    1.29 +In brief, we can summarize the guidelines as follows, and expand on them
    1.30 +in detail below:
    1.31 +<ul>
    1.32 +<li>follow the ns-3 coding style
    1.33 +<li>be licensed appropriately
    1.34 +<li>write associated documentation, tests, examples
    1.35 +<li>include a short description with each submission
    1.36 +<li>minimize the size of each submission
    1.37 +<li>changes must be coherent!
    1.38 +<li>split your code into multi-part submissions
    1.39 +<li>before sending a submission: ask yourself <i>if I were a reviewer, would I understand the submission?</i>
    1.40 +<li>pick a submission tool:
    1.41 +  <ul>
    1.42 +    <li>a simple patch
    1.43 +    <li>a binary bundle
    1.44 +    <li>a hosted clone
    1.45 +    <li>patch emails
    1.46 +  </ul>
    1.47 +<li>consider rewriting your development history
    1.48 +</ul>
    1.49 +<p>
    1.50 +If you do not have the time to follow through the process to include your
    1.51 +code in the main tree, please see <a href="#out-of-tree">below</a> about contributing
    1.52 +ns-3 code that is not maintained in the main tree.
    1.53 +</p>
    1.54 +
    1.55 +<h2>Licensing and coding style</h2>
    1.56 +
    1.57 +<h3>Licensing, copyright, and authorship</h3>
    1.58 +
    1.59 +<p>
    1.60 +All code submitted must conform to the project licensing framework, which
    1.61 +is GNU GPLv2 compatibility.  All new source files must contain a 
    1.62 +license statement.  All modified source files must either conform to
    1.63 +the license of the original file or (in rare cases) may add a 
    1.64 +license to the original license.  
    1.65 +
    1.66 +<p>Note that it is incumbent upon the submitter to make sure that the licensing
    1.67 +attribution is correct and that the code is suitable for ns-3 inclusion.
    1.68 +<i>Do not</i> include code (even snippets) from sources that have incompatible
    1.69 +licenses.
    1.70 +
    1.71 +<p>If you are writing a new file or making large modifications (more than 20 or 30 lines)
    1.72 +to an existing file, include a copyright statement in the header, such as: 
    1.73 +<tt>Copyright 2009 John Doe</tt>. Do not use the phrase <tt>All rights reserved.</tt>.
    1.74 +Optionally, you may also add a comment to help us track you down in case of problems in
    1.75 +the code such as <tt>Author:  john_doe@example.edu</tt>. However, do not create a chain of authors by
    1.76 +appending to this list in the header such as <tt>Modified by Jane Doe 02/04/09</tt>;
    1.77 +we use Mercurial change history to log who is changing what file.
    1.78 +
    1.79 +<p>You may append any contributors to the <tt>AUTHORS</tt> file as you see fit.
    1.80 +
    1.81 +<h3>Coding style</h3>
    1.82 +
    1.83 +<p>
    1.84 +Whatever code you submit, make sure that you follow the ns-3 <a href="http://www.nsnam.org/codingstyle.html">coding style</a>.
    1.85 +
    1.86 +<p>Some of the rules it describes might seem arbitrary to you and you
    1.87 +might disagree with these rules because the resulting code layout
    1.88 +looks ugly to you.  Even so, we ask you to consider carefully the 
    1.89 +purpose of a coding style: the purpose is to ensure that the overall
    1.90 +codebase is consistently indented and laid out to make it easier for a 
    1.91 +trained eye to deal with large amounts of code. 
    1.92 +
    1.93 +<p>So, whether you disagree with our layout rules, or you are unsure
    1.94 +about whether a specific piece of code matches the rules, focus
    1.95 +on making sure that the coding style of your code matches the coding 
    1.96 +style of any surrounding ns-3 code.
    1.97 +
    1.98 +<p>Of course, minor style errors and uncertainties can be easily 
    1.99 +corrected during reviews, but code submissions with a widescale 
   1.100 +ignorance of the ns-3 coding style will likely be bounced back until 
   1.101 +this step is taken.
   1.102 +
   1.103 +<h3>Whitespace changes</h3>
   1.104 +
   1.105 +<p>Do not ever submit code which contains whitespace/tab changes mixed
   1.106 +with other non-whitespace changes. Generally, it's a good idea to avoid
   1.107 +submitting whitespace changes at all, but, if you really must, submit
   1.108 +a single change consisting only of whitespace changes.
   1.109 +
   1.110 +<h2>Completing your code submission</h2>
   1.111 +
   1.112 +<p>Often we have observed that contributors will provide nice new classes
   1.113 +implementing a new model, but will fail to include supporting test
   1.114 +code, example scripts, and documentation.  Therefore, we ask people
   1.115 +submitting the code (who are in the best position to do this type of
   1.116 +work) to provide documentation, test code, and example scripts.
   1.117 +
   1.118 +<p>Note that you may want to go through multiple phases of code reviews,
   1.119 +and all of this supporting material may not be needed at the first stage (e.g.
   1.120 +when you want some feedback on public API header declarations only, before
   1.121 +delving into implementation).  However, when it times come to merge your
   1.122 +code, you should be prepared to provide these things, as fits your
   1.123 +contribution (maintainers will provide some guidance here).
   1.124 +
   1.125 +<h3>Including examples</h3>
   1.126 +
   1.127 +<p>For many submissions, it will be important to include at least one example
   1.128 +that exercises the new code, so the reviewer understands how it is intended
   1.129 +to be used.  For final submission, please consider to add as many examples
   1.130 +as you can that will help new users of your code.  The <tt>samples/</tt> and <tt>examples/</tt>
   1.131 +directories are places for these files.
   1.132 +
   1.133 +<h3>Testing</h3>
   1.134 +
   1.135 +<p>Every new code should include automated tests.  These should, where 
   1.136 +appropriate, be unit tests of model correctness, and may be validation
   1.137 +tests of stochastic behavior.  The test code should try to cover as 
   1.138 +much model code as possible, and should be checked as much as possible
   1.139 +by regression tests.  The project is still refining the testing framework
   1.140 +and will add more detail to this section later, but suffice it to say
   1.141 +for now that new features should come with test code to match; otherwise
   1.142 +it risks falling into disrepair over time.  
   1.143 +
   1.144 +<p>
   1.145 +TODO:  Add a reference (perhaps a manual chapter), with some examples and 
   1.146 +samples, of how to write good ns-3 tests.
   1.147 +
   1.148 +<h3>Documentation</h3>
   1.149 +
   1.150 +<p>If you add a new features, or make changes to existing features, you
   1.151 +need to update existing or write new documentation and example code.
   1.152 +Consider the following checklist:
   1.153 +<ul>
   1.154 +  <li>doxygen should be added to the header files, and should be checked
   1.155 +    for doxygen errors
   1.156 +  <li>incompatible API changes must be documented in <tt>CHANGES.html</tt>
   1.157 +  <li>new features should be described in <tt>RELEASE_NOTES</tt>
   1.158 +  <li>new API or changes to existing API must update the inline doxygen
   1.159 +    documentation in header files
   1.160 +  <li>consider updating or adding a new section to the tutorial in <tt>doc/tutorial/</tt>
   1.161 +  <li>consider updating or adding a new section to the manual in <tt>doc/manual/</tt>
   1.162 +  <li>update the <tt>AUTHORS</tt> list as appropriate
   1.163 +</ul>
   1.164 +
   1.165 +<h2>Preparing your code for review</h2>
   1.166 +
   1.167 +<h3>Description</h3>
   1.168 +
   1.169 +<p>For each code submission, include a description of what your code is
   1.170 +doing, and why. Ideally, you should be able to provide a summary
   1.171 +description in a 5-line paragraph with a 1-line (15 word) subject. If
   1.172 +you fix a bug filed in our <a href="http://www.nsnam.org/bugzilla">bugzilla database</a>, 
   1.173 +the subject should include first the bug number, and then, the bug title.
   1.174 +For example: <tt>bug 558: qos-tag.h is gone missing from wscript</tt>
   1.175 +
   1.176 +<p>Bonus points go to submitters who provide a description of their 
   1.177 +testing strategy.
   1.178 +
   1.179 +<h3>Minimize the size of each submission</h3>
   1.180 +
   1.181 +<p>Each submission should be as small as possible. Ideally, each submission
   1.182 +should deal with one issue only. If your description of your submission 
   1.183 +includes words such as <i>and</i>, it is a big red warning sign that you
   1.184 +should think about splitting your submission in two separate smaller 
   1.185 +submissions, if the changes are not intertwined.
   1.186 +
   1.187 +<h3>Coherent changes</h3>
   1.188 +
   1.189 +<p>Each submission should be a coherent whole: if you need to edit ten
   1.190 +files to get a feature to work, then, the submission should contain all
   1.191 +the changes for these ten files. Of course, if you can split the feature 
   1.192 +in sub-features, then, you should do it to decrease the size of the 
   1.193 +submission as per the previous item.
   1.194 +
   1.195 +<p>For example, if you have made changes to optimized a module and to fix
   1.196 +a bug in another module, make sure you separate these two sets of changes
   1.197 +in two separate submissions.
   1.198 +
   1.199 +<h3>Multi-part submissions</h3>
   1.200 +
   1.201 +<p>If you are working on a large new feature or a large refactoring, and 
   1.202 +because you will attempt to minimize the size of your submissions, you
   1.203 +will have to split your large work in multiple separate submissions.
   1.204 +
   1.205 +<p>Ideally, these submissions will be started by a detailed explanation
   1.206 +of the overall plan such that code reviewers can review each submission
   1.207 +separately but within a large context. This kind of work typically is
   1.208 +split in multiple dependent steps where each step depends on the previous
   1.209 +one. If this is the case, make it very clear in your initial explanation.
   1.210 +If you can, minimize dependencies between each step such that reviewers
   1.211 +can merge each step separately without having to consider the impact of
   1.212 +merging one submission on other submissions.
   1.213 +
   1.214 +<h3>Before sending a submission</h3>
   1.215 +
   1.216 +<p>When you send a submission for review and merging in ns-3, before
   1.217 +you hit the 'Send' button of your email client, ask yourself one 
   1.218 +last time: <i>If I were a reviewer, and I had to review that submission,
   1.219 +what would I do ?</i>. Specifically, make sure that you have provided 
   1.220 +enough context to allow someone else to understand what you did and why.
   1.221 +Keep in mind that your reviewer does not have access to a readable
   1.222 +dump of your brain: he has access only to your code, and your emails.
   1.223 +
   1.224 +<h3>Submission tools</h3>
   1.225 +
   1.226 +<p>
   1.227 +There are many ways to submit a piece of code. Some of them are not
   1.228 +appropriate:
   1.229 +<ul>
   1.230 +  <li>do not send us a modified version of each file you have changed
   1.231 +  <li>do not send us a .zip or .tar.gz file by email with a copy of
   1.232 +every modified file
   1.233 +  <li>do not send us a patch against an unspecified version of ns-3
   1.234 +</ul>
   1.235 +
   1.236 +<p>Others are more appropriate:
   1.237 +<ol>
   1.238 +  <li>a simple patch
   1.239 +  <li>patch reviews at <a href="http://codereview.appspot.com">codereview</a>
   1.240 +  <li>a binary bundle
   1.241 +  <li>a hosted clone
   1.242 +  <li>patchbombs
   1.243 +</ol>
   1.244 +
   1.245 +<ol>
   1.246 +<li><b>a simple patch</b>
   1.247 +
   1.248 +<p>Patches can be attached to a bug report or sent to our developer mailing-lists.
   1.249 +
   1.250 +<p>The UNIX <tt>diff</tt> tool is the most common way of producing a patch: a 
   1.251 +patch is a text-based representation of the difference between
   1.252 +two text files or two directories with text files in them. If you have
   1.253 +two files, <tt>original.cc</tt>, and, <tt>modified.cc</tt>, you can generate a patch
   1.254 +with the command <tt>diff -u original.cc modified.cc</tt>. If you wish to
   1.255 +produce a patch between two directories, use the command 
   1.256 +<tt>diff -uprN original modified</tt>.
   1.257 +
   1.258 +<p>Make sure you specify to the reviewer where the original files came 
   1.259 +from and make sure that the resulting patch file does not contain 
   1.260 +unexpected content by performing a final inspection of the patch 
   1.261 +file yourself.
   1.262 +
   1.263 +<p>Patches such as this are sufficient for simple bug fixes or very simple
   1.264 +small features.
   1.265 +
   1.266 +<li><b>patch reviews at <a href="http://codereview.appspot.com">codereview</a></b>
   1.267 +
   1.268 +<p>The best way to submit a larger patch for consideration is to request 
   1.269 +a patch review:
   1.270 +
   1.271 +<ol>
   1.272 +<li>download <a href="http://codereview.appspot.com/static/upload.py">upload.py</a>
   1.273 +<li>record the changes you want to request a review for in a mercurial 
   1.274 +repository: <tt>hg commit ...</tt>
   1.275 +<li>within the mercurial repository, run upload.py. Make sure you specify 
   1.276 +<tt>ns-3-reviews@googlegroups.com</tt> as a CC.
   1.277 +<li>announce your review request on ns-developers 
   1.278 +</ol>
   1.279 +
   1.280 +<p>When you send a tree without a detailed summary of your changes, it 
   1.281 +would help if you could send a list of the changesets you want to merge. 
   1.282 +To generate it, first merge with ns-3-dev and then, from your modified 
   1.283 +directory, run <tt>hg outgoing -p http://code.nsnam.org/ns-3-dev</tt>
   1.284 +
   1.285 +<p>If you already pulled changes from ns-3-dev into your private repository 
   1.286 +after you started doing your private modifications, there is an issue to 
   1.287 +be considered. upload.py does not let you specify a range of revisions, 
   1.288 +nor a set of changesets. So if you just run <tt>upload.py</tt> in your private 
   1.289 +repository, the changesets pulled from ns-3-dev will also be published 
   1.290 +on codereview, which is, of course, not desirable.
   1.291 +
   1.292 +<p>A possible workaround is to pull your changes into a temporary 
   1.293 +repository which is an up-to-date clone of ns-3-dev. The following code 
   1.294 +should do the trick. This code assumes that your private repository is in 
   1.295 +path DEV_BRANCH_WITH_NEW_FEATURE, and that it is in sync with ns-3-dev.
   1.296 +
   1.297 +<pre>
   1.298 +hg clone http://code.nsnam.org/ns-3-dev ns-3-tmp
   1.299 +cd ns-3-tmp
   1.300 +export REVNO=`hg tip -q | sed 's/:.*$//'`
   1.301 +hg pull DEV_BRANCH_WITH_NEW_FEATURE
   1.302 +hg merge
   1.303 +hg commit -m "merged new feature"
   1.304 +upload.py --rev=$REVNO --cc=ns-3-reviews@googlegroups.com 
   1.305 +</pre>
   1.306 +
   1.307 +<li><b>Binary bundles</b>
   1.308 +
   1.309 +
   1.310 +<p>ns-3 uses <a href="http://www.selenic.com/mercurial/">mercurial</a> as its 
   1.311 +version control tool: a quick introduction to using it can be found on our 
   1.312 +<a href="http://www.nsnam.org/mercurial.html.">homepage</a>.
   1.313 +Further extensive documentation and manuals are available from the mercurial
   1.314 +<a href="http://www.selenic.com/mercurial/">homepage</a>.
   1.315 +
   1.316 +<p>If you get used to use mercurial to record your daily/hourly work using
   1.317 +mercurial in a local clone of the ns-3 main repository, and if you want to 
   1.318 +share that work with other people to get initial feedback about your work,
   1.319 +your implementation approach, and start a high-level discussion about it,
   1.320 +you can easily package all your local work history as a single binary bundle
   1.321 +file with the <tt>hg bundle</tt> command and send this file over email. 
   1.322 +
   1.323 +<li><b>Hosted clones</b>
   1.324 +
   1.325 +
   1.326 +<p>If your bundles grow large in size, it can become tricky to send them over
   1.327 +email: hosting a clone of your mercurial repository is a simple way to work
   1.328 +around this. If you have a host with a static ip address and/or a hostname,
   1.329 +you can run <tt>hg serve</tt> within your repository in this host and send instead
   1.330 +to your co-workers a url to that repository: if I run <tt>hg serve -d -p 8080</tt> 
   1.331 +on my local ns-3-dev clone, I could send the url <tt>http://diese.inria.fr:8080/</tt>
   1.332 +to my co-workers to allow them to pull all my development history from there.
   1.333 +
   1.334 +<p>If you don't have a static ip address or hostname, or are located behind
   1.335 +a NAT or a firewall, you have to use a third-party hosting provider for your
   1.336 +code. A small list of hosting providers is available 
   1.337 +<a href="http://www.selenic.com/mercurial/wiki/index.cgi/MercurialHosting">there</a>
   1.338 +
   1.339 +
   1.340 +<p>We can also provide hosting to ns-3 developers on <a href="http://code.nsnam.org/">code.nsnam.org</a>
   1.341 +should none of the above alternative work for you.
   1.342 +
   1.343 +<li><b>Patchbombs</b>
   1.344 +
   1.345 +<p>Patchbombs are a nice way to perform a request for a detailed
   1.346 +review. Make sure you enable the patchbomb extension in your ~/.hgrc:
   1.347 +<pre>
   1.348 +[extensions]
   1.349 +hgext.patchbomb = 
   1.350 +</pre>
   1.351 +Make sure you configure your patchbomb extension correctly: 
   1.352 +<tt>man 5 hgrc</tt> and look in the [email] and [smtp] sections
   1.353 +
   1.354 +Finally:
   1.355 +<pre>
   1.356 +hg email -i -t ns-developers@isi.edu http://code.nsnam.org/ns-3-dev
   1.357 +</pre>
   1.358 +
   1.359 +<p>Beware that generating too many emails with this tool without doing
   1.360 +proper <i>history rewriting</i> to produce clean easy-to-review patches will
   1.361 +be considered rude.
   1.362 +
   1.363 +</ol>
   1.364 +
   1.365 +<h3>History rewriting</h3>
   1.366 +
   1.367 +<p>The idea behind history rewriting is to request a code submitter to 
   1.368 +re-arrange their repository history prior to merging in the main
   1.369 +ns-3 repository. 
   1.370 +
   1.371 +<p>For example, let's say that during development of a new feature, you
   1.372 +decide to use your mercurial repository as a fancy version-enabled
   1.373 +backup system: you do a lot of work, and regularly commit it to save it.
   1.374 +When you are done implementing and testing that new feature, the resulting
   1.375 +repository history as shown with <tt>hg log</tt> will look very verbose, and
   1.376 +will most likely contain a lot of commit messages such as <i>fix bug</i>.
   1.377 +Clearly, none of these commits are very helpful and there is little point
   1.378 +in keeping them around: they make it painful to use the annotate, and bisect
   1.379 +commands, and will make review of the final mercurial tree harder than it
   1.380 +need to be.
   1.381 +
   1.382 +<p>A simple way to work around these problems is to ask each contributor to
   1.383 +re-structure their commit history before submitting their tree for review:
   1.384 +<ul>
   1.385 +  <li>pointless backup commits are deleted from the history
   1.386 +  <li>commits are re-organized in sub-feature commits
   1.387 +  <li>each commit is made buildable
   1.388 +  <li>etc.
   1.389 +</ul>
   1.390 +
   1.391 +<ol>
   1.392 +<li><b>Rewrite a new history from scratch</b>
   1.393 +
   1.394 +<p>Once you are done with a new feature, you can generate one final patch
   1.395 +with the mercurial <tt>diff</tt> command and apply it to a clean repository
   1.396 +as one single commit with a nice new commit message.
   1.397 +
   1.398 +<p>A variant on the above is to split the final commit in multiple smaller
   1.399 +commits, each of which addresses one aspect of the final feature and
   1.400 +make sure that each commit is still buildable.
   1.401 +
   1.402 +<li><b>Build clean history from the start</b>
   1.403 +
   1.404 +<p>Instead of using mercurial as a powerful backup system and trying to go back
   1.405 +later, some users prefer to use tools such as 'mq' or 'pbranch' to split
   1.406 +their work from the start in a set of separate entities and still record 
   1.407 +somewhere their day-to-day development history.
   1.408 +
   1.409 +<p>More information on these tools:
   1.410 +<ul>
   1.411 +<li><a href="http://hgbook.red-bean.com/read/managing-change-with-mercurial-queues.html">Managing change with mercurial queues</a>
   1.412 +<li><a href="http://hgbook.red-bean.com/read/advanced-uses-of-mercurial-queues.html">Advanced uses of mercurial queues</a>
   1.413 +<li>The <a href="http://arrenbrecht.ch/mercurial/pbranch/">Pbranch</a> extension.
   1.414 +</ul>
   1.415 +
   1.416 +</ol>
   1.417 +
   1.418 +<h2>Submitting out-of-tree code</h2>
   1.419 +
   1.420 +<p>TODO:  Write about how contributors can send a tarball 
   1.421 +for archiving at code.nsnam.org if they do not want to go through
   1.422 +the process for main-tree code.  Write about src/contrib.
     2.1 --- a/nsnam.css	Tue Apr 14 09:07:28 2009 +0200
     2.2 +++ b/nsnam.css	Wed May 06 14:55:46 2009 +0200
     2.3 @@ -28,6 +28,12 @@
     2.4     color: #0047b9;
     2.5  }
     2.6  
     2.7 +tt {
     2.8 +   font-size: 11pt;
     2.9 +   background: #e0e0e0;
    2.10 +   color: black;
    2.11 +}
    2.12 +
    2.13  img {
    2.14      border: 0px;
    2.15  }