|
Size: 4837
Comment:
|
← Revision 10 as of 2010-11-10 11:42:03 ⇥
Size: 31
Comment: content rolled into ContributingChanges
|
| Deletions are marked like this. | Additions are marked like this. |
| Line 1: | Line 1: |
| == A successful Patch is... == ''(from a posting on 2008-10-03 by mpm to [[http://selenic.com/pipermail/mercurial-devel/2008-October/008279.html|mercurial-devel]])'' '''a) in the BODY of the message''' If I can't hit reply to your patch, I'm not going to go to the trouble of cutting and pasting your code from a URL to give you feedback. I have too much to do. Likewise, if I can't drag and drop your message into my patch queue, I'll probably postpone dealing with it effectively forever. Patches sitting in the [[http://selenic.com/mercurial/bugs/|BTS]] or elsewhere are dead on arrival. Our backlog is too long for us to deal with the stuff that's in our inboxes, let alone elsewhere. '''b) broken into digestible chunks''' We don't want patches that radically change things. They're more likely to break things and they can't be bisected. Also, they're too big to review. And we obviously can't take patches we can't review: if the quality of the code base drops, we'll have even less time to deal with contributions. Ideally we want a series of patches that each make one small step towards the goal. Each patch should leave the code in a working state and take less than five minutes to look over. Again, if it takes me more than a few minutes to deal with, it'll go into the backlog. If you can, put the least controversial bits first. If you're lucky, someone will apply those right away and you'll be that much closer to being done. '''c) include all the relevant context''' If you're resending a patch that I commented on last week, it's safe to assume I've forgotten everything about it. In the intervening times I've looked at dozens of other patches and bugs, spent lots of time working on unrelated projects, and maybe gone to a conference. So if your patch says "here is a resend of the patch from last week with some changes", I will have absolutely no idea what that means. Rather than grovelling through thousands of messages in my received mail folder to figure out what your patch is about, I'll probably just put your patch on the "to look at later" pile. So tell me everything you want me to know about your patch. Every time. Preferably in the patch's changelog. I want to know how it works, what the new output looks like, why you did it the way you did and not some other way, *what feedback you've incorporated*, etc. If your patch is a reasonable size, this should still be a short message and I should be able to read the whole thing and reply to it or apply it in five minutes. '''d) try not to overwhelm us''' If you're not being successful with getting your entire series of patches in all at once, you may be more successful sending just the first small pieces. You're more likely to make headway, and we'll have a lot less mail. You'll also find it's very helpful to talk to us on IRC about getting your patch in. We can often give you realtime feedback on any blocking issues. '''e) testing with the test suite''' (added by DirkjanOchtman) Make sure the test suite still passes after applying your patch. In most cases, adding a new test or appending to an old test is also a good thing. If we apply your patch and something breaks in the test suite, that's obviously a problem. Adding new tests for your newly added feature or bugfix will help us assure that it will keep working, freeing up more time not spent hunting bugs. == How to describe your successful patch == These points are particularly valuable when implementing a major new feature, but it's certainly a good checklist to keep in mind while writing your patch description. We'd like to know: * why we need this patch * how you've implemented it * what file formats and data structures you've used * what choices you've made * why the choices you've made are the right ones * why the choices you didn't make are the wrong ones * what shortcomings exist * what compatibility issues exist * what's missing, if anything == Why we want one feature per patch == ''(from a posting on 2008-08-22 by mpm to [[http://www.selenic.com/pipermail/mercurial-devel/2008-August/007632.html|mercurial-devel]])'' 1. '''The probability of a patch being acceptable''' is the product of the probability of each individual feature being acceptable. Which means it's a good idea to submit each feature separately. 2. '''The probability of there being a bug in the patch''' is the sum of there being a bug in each feature. Which means you want each feature in a separate commit for locating regressions. 3. '''The difficulty of reviewing a patch''' increases more than linearly with the number of features included. Which means we really don't want to review patches with more than one feature. ---- CategoryDeveloper |
#redirect ContributingChanges |
