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