www/patch-submission.html

   1 <html>
   2 <head>
   3 <title>Patch Submission</title>
   4 </head>
   5 <body>
   6
   7 <div id="apphead">
   8 <h1><small>scons</small><br />Patch Submission</h1>
   9 </div>
  10
  11 <p>
  12 <strong>You must now
  13 <a href="http://www.tigris.org/servlets/Login">log in</a>
  14 to a <a href="http://www.tigris.org">tigris.org</a> account
  15 before submitting a patch!</strong>
  16 </p>
  17
  18 <p>
  19 Patches should be submitted to the
  20 <a href="http://scons.tigris.org/issues/enter_bug.cgi?component=scons&subcomponent=scons&issue_type=PATCH">"Enter Issue" page</a>.
  21 Please follow the <a href="patch-submission.html#guidelines">submission guidelines</a> below
  22 to make sure your patch contains the necessary information.
  23 A more detailed set of <a href="patch-submission.html#steps">submission steps</a>
  24 can be found below.
  25 </p>
  26
  27 </div>
  28
  29 <div class="h2 app" style="border-left: 0px" id="customcontent">
  30
  31 <h2 id="guidelines">Guidelines for Patch Submission</h2>
  32
  33 <p>
  34 To try to maintain and improve the quality of SCons releases,
  35 we have some pretty high standards for the quality of patches
  36 that make it into the SCons code base.
  37 This list of guidelines describes how to make it as
  38 easy as possible for your patch to be accepted for integration.
  39 We're still interested in your code
  40 even if you don't follow all of these guidelines,
  41 but then your patch will more than likely sit in the queue
  42 until someone else has time to supply all of the
  43 necessary missing items.
  44 </p>
  45
  46 <ul>
  47
  48 <li>
  49 <strong>
  50 Please
  51 <a href="http://www.tigris.org/servlets/Login">log in</a>
  52 to your tigris.org account before submitting any patch
  53 </strong>
  54 <p>
  55 If you do not already have a tigris.org account,
  56 register for one at
  57 <a href="http://www.tigris.org/servlets/Join">http://www.tigris.org/servlets/Join</a>.
  58 </p>
  59 <p>
  60 We no longer accept anonymous patches,
  61 due to spambot abuse of the open-door policy.
  62 </p>
  63 </li>
  64
  65 <li>
  66 <strong>If your patch is extensive, discuss it first on the
  67 <a href="mailto:dev@scons.tigris.org">dev@scons.tigris.org</a>
  68 mailing list
  69 </strong>
  70 <p>
  71 In fact, for extensive changes, it's a good idea to have this discusssion
  72 <em>before</em> you invest too much time in coding.
  73 It's possible that your idea overlaps with something else
  74 already in the works,
  75 or that your idea is unlikely to be accepted
  76 because it would conflict with planned directions for SCons.
  77 It's much better to find that out,
  78 or get advice on acceptable design choices.
  79 before you've spent a lot of time polishing code
  80 that will be rejected because it doesn't fit
  81 plans for the architecture.
  82 </p>
  83 </li>
  84
  85 <li>
  86 <strong>It's better to submit multiple patches with separate bits of functionality than a big patch containing lots of changes</strong>
  87 <p>
  88 Big, intertwined sets of changes
  89 increase the chances of unintended side effects
  90 that could cause the entire patch to be rejected.
  91 If you submit separate functional changes in separate patches,
  92 those change that meet all the criteria can
  93 still be integrated even
  94 though other pieces might be held up for one reason or another.
  95 </p>
  96 </li>
  97
  98 <li>
  99 <strong>Submit your patch in <tt>diff -u</tt> or <tt>diff -c</tt> format</strong>
 100 <p>
 101 In particular, do <em>not</em> submit whole source files,
 102 or <tt>diff</tt> output without any kind of context information.
 103 It's much more difficult to integrate whole source files
 104 or plain <tt>diff</tt> output with other changes to
 105 the SCons code base,
 106 especially other changes that might be integrated
 107 after you've submitted your patch.
 108 </p>
 109 </li>
 110
 111 <li>
 112 <strong>Your patch must include test case(s) before it can be integrated!</strong>
 113 <p>
 114 THIS IS THE SINGLE MOST COMMON REASON FOR DELAYS IN INTEGRATING PATCHES
 115 AND THE SINGLE MOST IMPORTANT THING YOU CAN DO TO INCREASE THE
 116 CHANCES OF YOUR PATCH BEING INTEGRATED QUICKLY.
 117 </p>
 118 <p>
 119 The SCons development methodology requires
 120 that each change be accompanied by one or more
 121 new or modified test cases
 122 that get added to our extensive regression test suite.
 123 This is to make sure that the behavior added by your patch
 124 doesn't get inadvertently broken by other changes in the future.
 125 Patches that fix bugs should contain at least one test case
 126 that demonstrates the behavior being fixed by the patch.
 127 For example, if you're fixing a configuration that causes
 128 SCons to exit with an error and a stack trace,
 129 the test case should trigger that stack trace
 130 when run against the current code.
 131 Patches that add new features or enhancements
 132 should contain test cases that use
 133 the new behavior being added to SCons.
 134 </p>
 135 <p>
 136 You can do any of the following to supply
 137 test cases with your patch:
 138 </p>
 139 <ul>
 140 <li>
 141 <strong>Include actual new or modified SCons test scripts in your patch</strong>
 142 <p>
 143 This is the best option because it's the easiest to integrate,
 144 and therefore maximizes the chances of your patch being accepted quickly.
 145 (Note that, yes, there's a curve to learning how to
 146 write test scripts in the SCons testing harness.
 147 We're working on documentation to deal with that.)
 148 </p>
 149 </li>
 150 <li>
 151 <strong>Include a .tar.gz or .zip file containing test configurations</strong>
 152 <p>
 153 If you can't quite figure out how to deal with the SCons test scripts,
 154 the next best option is to include with your patch an archive file
 155 containing one or more actual test configurations
 156 (<tt>SConscript</tt> files, input files, etc.).
 157 It will be relatively straightforward for someone integrating your patch,
 158 and who's presumably familiar with the SCons testing harness,
 159 to turn this into an appropriate test script.
 160 Be sure to include a description of how to run your recommended test scenario,
 161 or a script for doing so.
 162 </p>
 163 </li>
 164 <li>
 165 <strong>Describe how to go about testing the patch</strong>
 166 <p>
 167 If you really can't cook up a test configuration to include with the patch,
 168 the lowest-common-denominator approach is to just describe
 169 how to go about testing the patch.
 170 Be as specific as possible,
 171 even if you think it should be obvious
 172 how to test the patch.
 173 It might be clear to you while you're writing the code,
 174 but it will still take someone else time
 175 to make sure they understand your intent
 176 and work out the details of how to set up an appropriate case.
 177 The point is you're trying to use your existing knowledge
 178 of the bug being fixed or new feature being added
 179 to make the process of integrating your patch as
 180 simple and quick as possible,
 181 thereby increasing the chance of your patch making it
 182 into the SCons code base.
 183 </p>
 184 </li>
 185 </ul>
 186 <p>
 187 If you don't supply <em>any</em> sort of testing
 188 information with your patch,
 189 well, you're still welcome to submit the code.
 190 Just be aware that the patch will likely stay
 191 in the queue until someone has time to reverse-engineer
 192 a test case.
 193 </p>
 194 </li>
 195
 196 <li>
 197 <strong>Your patch should not break any existing tests</strong>
 198 <p>
 199 This almost sounds like it should go without saying,
 200 but the reason we put so much emphasis on test cases
 201 is so that we can make sure functionality doesn't break.
 202 Your patch will almost certainly be run through the
 203 the complete set of checked-in test scripts,
 204 and if any of them break,
 205 your patch will either be rejected outright
 206 or delayed while someone else figures out how to fix it
 207 (or the tests) so that everything works correctly.
 208 You should, of course, avoid this by running your patch
 209 against the regression tests and fixing any problems
 210 <em>before</em> submitting your patch.
 211 If you run your patch against against the regression tests
 212 but can't figure out how to fix all the cases,
 213 the best bet would be to ask the
 214 <a href="mailto:dev@scons.tigris.org">dev@scons.tigris.org</a>
 215 mailing list.
 216 </p>
 217 </li>
 218
 219 <li>
 220 <strong>Your patch should include documentation changes</strong>
 221 <p>
 222 We also insist that changes to the SCons code base
 223 be accompanied by appropriate changes to the documentation.
 224 In practice, right now we make sure the man page is up to date,
 225 and updates to the User's Guide often lag.
 226 </p>
 227 <p>
 228 Similar to the guidelines above for testing,
 229 if you don't submit changes to the actual man page with your patch,
 230 it's helpful if you at least provide
 231 some suggested text describing your change.
 232 Even if the actual words get rewritten
 233 (usually to make the style consistent with the rest of the man page),
 234 taking the time to provide this
 235 makes the integration easier because
 236 the person integrating the patch doesn't have
 237 to reverse-engineer the <em>intent</em>
 238 of your change to figure out how to describe it.
 239 </p>
 240 </li>
 241
 242 </ul>
 243
 244 <h2 id="steps">Steps for Submitting a Patch</h2>
 245
 246 <p>
 247 The following guides you step-by-step through the
 248 process of submitting a patch to SCons.
 249 </p>
 250
 251 <p>
 252 NOTE:  Attaching a file or files
 253 (such as a .tar.gz or .zip file containing your patch)
 254 is a two-step process in the tigris.org Issue Tracker.
 255 You must first create the patch issue in the Tracker,
 256 and then attach the file(s) in a separate step,
 257 as described below.
 258 </p>
 259
 260 <ul>
 261
 262 <li>
 263 <strong><a href="http://www.tigris.org/servlets/Login">Log in</a> at tigris.org</strong>
 264 <p>
 265 If you do not already have a tigris.org account,
 266 register for one at
 267 <a href="http://www.tigris.org/servlets/Join">http://www.tigris.org/servlets/Join</a>.
 268 </p>
 269 <p>
 270 We no longer accept anonymous bug reports,
 271 due to spambot abuse of the open-door policy.
 272 </p>
 273 </li>
 274
 275 <li>
 276 <strong>Go to the
 277 <a href="http://scons.tigris.org/issues/enter_bug.cgi?component=scons&subcomponent=scons&issue_type=PATCH">"Enter issue" page</a>
 278 </strong>
 279 <p>
 280 By default, the "scons" subcomponent is selected;
 281 if this bug is for a different subcomponent, select that instead.
 282 </p>
 283 </li>
 284
 285 <li>
 286 <strong>Specify the version of SCons that you used as a baseline</strong>
 287 <p>
 288 You can leave this <tt>-unspecified-</tt>,
 289 in which case the assumption will be that you started with
 290 the code checked in to our Subversion repository
 291 at the time you opened the issue.
 292 </p>
 293 </li>
 294
 295 <li>
 296 <strong>Fill in a good Summary line describing the patch</strong>
 297 <p>
 298 This line is what shows up in summary reports,
 299 so it should be descriptive but not too long.
 300 Avoid overly-general things like "SCons error," etc.
 301 </p>
 302 </li>
 303
 304 <li>
 305 <strong>Fill in the Description field</strong>
 306 <p>
 307 This is where you should describe
 308 the nature of your patch:
 309 the exact error it fixes,
 310 the feature that it adds,
 311 how to go about testing it,
 312 etc.
 313 When in doubt, include more information rather than less.
 314 </p>
 315 </li>
 316
 317 <li>
 318 <strong>Press the "Submit issue" to submit your report</strong>
 319 <p>
 320 You will now receive a <tt>Posting issue</tt> page
 321 that gives you the number of the issue you submitted.
 322 </p>
 323 </li>
 324
 325 <li>
 326 <strong>Click the "Attach a file to this issue" link</strong>
 327 <p>
 328 </p>
 329 </li>
 330
 331 <li>
 332 <strong>Fill in the "File" field with the path to the patch file you want to upload</strong>
 333 <p>
 334 (You can also do this through the <tt>Browse...</tt> button.)
 335 </p>
 336 </li>
 337
 338 <li>
 339 <strong>Fill in the Description field</strong>
 340 <p>
 341 </p>
 342 </li>
 343
 344 <li>
 345 <strong>Click the "Submit" button to attach your patch file</strong>
 346 <p>
 347 </p>
 348 </li>
 349
 350 </ul>
 351
 352 </div>
 353
 354 </body>
 355 </html>