Differences between revisions 40 and 41
Revision 40 as of 2010-11-11 04:26:32
Size: 6416
Editor: mpm
Comment:
Revision 41 as of 2010-12-02 17:50:42
Size: 6488
Comment: clarify patch description and email format
Deletions are marked like this. Additions are marked like this.
Line 3: Line 3:
Line 11: Line 10:
Line 17: Line 15:
 * Patch emails should have '''[PATCH]''' in the subject  * Patch emails should have '''[PATCH]''' in the subject followed by a summary (not included in the patch)
Line 19: Line 17:
 * Patches should be in the form output by '`hg export`' - unified diff with author and summary  * Patches should be in the '`# HG changeset patch`' form output by '`hg export`' - unified diff with author and full patch description
Line 23: Line 21:
Because this is a community project and our developers are very busy, patches will sometimes fall through the cracks. If you've gotten '''no response''' to your patch after a few days, feel free to resend it.  Because this is a community project and our developers are very busy, patches will sometimes fall through the cracks. If you've gotten '''no response''' to your patch after a few days, feel free to resend it.
Line 26: Line 24:
Line 37: Line 34:

* lowercase summary line, less than 80 characters, no trailing period 
 * lowercase summary line, no trailing period
Line 44: Line 40:
 * all lines less than 80 characters
Line 55: Line 52:
 * what's missing, if anything   * what's missing, if anything
Line 57: Line 54:
 
Line 59: Line 56:
Line 64: Line 60:
 * Run the [[WritingTests|test suite]] to make sure you haven't broken anything   * Run the [[WritingTests|test suite]] to make sure you haven't broken anything
Line 68: Line 64:
 
Line 70: Line 66:

If you're making a large change, we're probably going to want it broken into a ''series'' of smaller patches.
This makes for easier review and tests both for us and for you.
This can be tricky at first and you might find tools like [[MQ]] and [[RecordExtension|record]] useful in this process. 
If you're making a large change, we're probably going to want it broken into a ''series'' of smaller patches. This makes for easier review and tests both for us and for you.  This can be tricky at first and you might find tools like [[MQ]] and [[RecordExtension|record]] useful in this process.
Line 92: Line 85:
Line 109: Line 101:
Line 115: Line 106:
Line 122: Line 112:
Line 133: Line 122:

Contributing Changes

How to help us improve Mercurial's code.

/!\ This page is intended for (aspiring) developers.

1. The basics: patches by email

/!\ Don't send your patch to the BugTracker - it can't be reviewed there, so it won't go anywhere!

We like to receive changes as patches by email. This allows us to review patches, give feedback, and track which patches need attention.

  • Patches go to mercurial-devel@selenic.com - no subscription necessary!

  • Patch emails should have [PATCH] in the subject followed by a summary (not included in the patch)

  • We prefer patches in the message body so we can review them (no attachments or URLs!)

  • Patches should be in the '# HG changeset patch' form output by 'hg export' - unified diff with author and full patch description

  • Sending a patch implies granting permission to use it in our project under an appropriate license

  • Apple Mail, Gmail and possibly other mail clients corrupt patches when sending them in the body. Using the patchbomb extension always works; see below for details.

Because this is a community project and our developers are very busy, patches will sometimes fall through the cracks. If you've gotten no response to your patch after a few days, feel free to resend it.

2. Patch descriptions

It's important that you describe your patch. Patch descriptions should be in the following format:

opener: check hardlink count reporting (issue1866)

The Linux CIFS kernel driver (even in 2.6.36) suffers from a hardlink
count blindness bug (lstat() returning 1 in st_nlink when it is expected
to return >1), which causes repository corruption if Mercurial running
...
  • lowercase summary line, no trailing period
  • start with the most useful identifiable command or subsystem (not necessarily the module name!)
  • summarize the fix, not the problem
  • add '(issueNNNN)' to refer to a BugTracker issue (and automatically move the issue to testing)

  • a blank line after the summary
  • a more complete description of the problem if necessary
  • all lines less than 80 characters

Patch descriptions should be aimed at helping the reviewer understand the issue you're addressing. Try to answer the following, where appropriate:

  • why we need this patch
  • how you've implemented it
  • why the choices you've made are the right ones
  • why the choices you didn't make are the wrong ones
  • what all the corner cases are (consider a table!)
  • what shortcomings exist
  • what file formats and data structures you've used
  • what compatibility issues exist

  • what's missing, if anything
  • what it looks like, if relevant (include sample output!)

3. Coding style and testing

<!> If you send a patch with an underscore in a variable name, we'll know you haven't read this page!

  • See our coding style for what we expect code to look like (yes, we're serious about the underscores)

  • Use contrib/check-code.py to check for common style errors

  • Run the test suite to make sure you haven't broken anything

  • Add new test cases as needed (preferably by extending existing tests)
  • Patches should apply cleanly against the tip of the appropriate branch (default or stable)
  • Do not include unrelated changes or cleanups!

4. Organizing patches

If you're making a large change, we're probably going to want it broken into a series of smaller patches. This makes for easier review and tests both for us and for you. This can be tricky at first and you might find tools like MQ and record useful in this process.

Each patch in a series should:

  • implement one clear step in your process
  • leave the code in a working state (so bisect always works!)
  • include relevant test changes so they're independently testable
  • be sent as a separate email

<!> Do not mix formatting changes, organizational changes, or functional changes in the same patch!

Things to consider:

  • put the least controversial pieces first - if you're lucky, they'll get applied right away
  • the difficulty of reviewing a patch increases rapidly with size - small patches are more likely to get attention
  • the probability of a patch getting rejected is also a function of its size - smaller patches mean fewer review round-trips
  • bigger patches have more bugs - smaller patches make it easier to locate regressions

5. Mailer issues and patchbomb

As mentioned above, patches should be included in the body of emails to make it easy for developers to review and apply your patches. Some mailers make this difficult by mangling the whitespace in patches or similar. If you have trouble sending clean patches or are sending more than a couple patches, you should probably set up the patchbomb extensions which automates the process.

Add something like the following to your .hgrc:

[extensions]
patchbomb=

[email]
method = smtp
from = Your Name <your@email.address>
to = mercurial-devel@selenic.com

[smtp]
host = your.smtp.server.com

Then run the following to do a dryrun test:

$ hg email --test <change1> <change2> ...

Notable options:

  • '--flag' can be used to flag a patch, e.g. "STABLE", "RFC", "v2"

  • don't use '--inline', it's a weird MIME thing and not what we want

6. Etiquette and advice

  • Feel free to resend patches after a few days if you haven't gotten a response
  • Try to respond quickly to feedback before we forget what your patch is about
  • Tell us everything you want us to know about a patch, every time
  • If we ask for fixes, don't send a patch to your patch, send a new fixed patch
  • Don't overwhelm us - if you can't get all 10 patches reviewed, try the first 5
  • Consult 'hg log' to cc: the most relevant developers for the code you're working on

  • Consider giving input on other people's patches
  • Discuss your patches on IRC to get faster review and valuable initial feedback

7. See also


CategoryDeveloper

ContributingChanges (last edited 2023-04-06 07:59:14 by RaphaelGomes)