Thread overview | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
February 18, 2012 [phobos] Pull request style | ||||
---|---|---|---|---|
| ||||
Attachments:
| First off, thanks to everyone who is doing pull requests. This is a great help! I'm spamming all three lists, because I think this is general and helpful. 1. Commit messages: Please, please, PLEASE when a pull request fixes a bugzilla issue, use this as the first line of the commit message: fix Issue 5412 - import wtf2 Note that everything after the string "fix" is cut & pasted from the bugzilla title: http://d.puremagic.com/issues/show_bug.cgi?id=5412 Easy as pie. Doing it this way means that bugzilla and github automatically talk to each other about the fix. It makes life easier for the poor sod trying to keep the cross references up to date, give proper credit to the submitter, etc. It is not necessary to invent a new convention for this. Please don't. 2. The pull request often comes with a nice and lucid explanation about the fix, oddities about it, etc. But then the actual code patch has no comments at all. The code is where the explanation of how the fix works goes. It's nice to have it in the pull request too, but please cut&paste it into a comment in the right place in the code. If there's a long-winded discussion in bugzilla or the n.g. about how things should work, it can be nice to put a link to that discussion in the comments. 3. Please do not combine independent fixes into one pull request. Each fix should live in its own pull request. The problem with combining them is it's a darn nuisance if one of them isn't right. 4. It's nice if, in the pull request message, provide a link to the bugzilla entry it addresses. This makes it quick&easy for me to see what's up with the bug report. 5. You guys have been great about including test cases that verify the fix. Please continue! |
February 19, 2012 Re: [phobos] Pull request style | ||||
---|---|---|---|---|
| ||||
Posted in reply to Walter Bright | 1. Walter, what about pull requests for D1/D2 bugs? These don't completely fix the bug, they require a D1 patch before it can be closed. I didn't get a reply when I asked this before. (Sometimes I just forget) 2. I think this applies to commit messages too. It's really hard to work out _why_ someone made certain changes when tracing a bug back through git-blame. 5. I'd like that I try and cover as many cases as possible. Sometimes with a couple of extra lines you can use static foreach or templates to cover every integral type, or all operators, or 6 different types of declaration. On Sun, Feb 19, 2012 at 14:52, Walter Bright <walter@digitalmars.com> wrote: > First off, thanks to everyone who is doing pull requests. This is a great help! > > I'm spamming all three lists, because I think this is general and helpful. > > 1. Commit messages: > > Please, please, PLEASE when a pull request fixes a bugzilla issue, use this as the first line of the commit message: > > fix Issue 5412 - import wtf2 > > Note that everything after the string "fix" is cut & pasted from the bugzilla title: > > http://d.puremagic.com/issues/show_bug.cgi?id=5412 > > Easy as pie. Doing it this way means that bugzilla and github automatically talk to each other about the fix. It makes life easier for the poor sod trying to keep the cross references up to date, give proper credit to the submitter, etc. > > It is not necessary to invent a new convention for this. Please don't. > > > 2. The pull request often comes with a nice and lucid explanation about the fix, oddities about it, etc. But then the actual code patch has no comments at all. The code is where the explanation of how the fix works goes. It's nice to have it in the pull request too, but please cut&paste it into a comment in the right place in the code. If there's a long-winded discussion in bugzilla or the n.g. about how things should work, it can be nice to put a link to that discussion in the comments. > > > 3. Please do not combine independent fixes into one pull request. Each fix should live in its own pull request. The problem with combining them is it's a darn nuisance if one of them isn't right. > > > 4. It's nice if, in the pull request message, provide a link to the bugzilla entry it addresses. This makes it quick&easy for me to see what's up with the bug report. > > > 5. You guys have been great about including test cases that verify the fix. Please continue! > > _______________________________________________ > phobos mailing list > phobos@puremagic.com > http://lists.puremagic.com/mailman/listinfo/phobos _______________________________________________ phobos mailing list phobos@puremagic.com http://lists.puremagic.com/mailman/listinfo/phobos |
February 18, 2012 Re: [phobos] Pull request style | ||||
---|---|---|---|---|
| ||||
Posted in reply to Daniel Murphy | On 2/18/2012 8:39 PM, Daniel Murphy wrote: > 1. Walter, what about pull requests for D1/D2 bugs? These don't > completely fix the bug, they require a D1 patch before it can be > closed. I didn't get a reply when I asked this before. (Sometimes I > just forget) I generally just fold those into D1 myself. > 2. I think this applies to commit messages too. It's really hard to > work out _why_ someone made certain changes when tracing a bug back > through git-blame. > > 5. I'd like that I try and cover as many cases as possible. Sometimes > with a couple of extra lines you can use static foreach or templates > to cover every integral type, or all operators, or 6 different types > of declaration. Sure, use your best judgment. > On Sun, Feb 19, 2012 at 14:52, Walter Bright<walter@digitalmars.com> wrote: >> First off, thanks to everyone who is doing pull requests. This is a great >> help! >> >> I'm spamming all three lists, because I think this is general and helpful. >> >> 1. Commit messages: >> >> Please, please, PLEASE when a pull request fixes a bugzilla issue, use this >> as the first line of the commit message: >> >> fix Issue 5412 - import wtf2 >> >> Note that everything after the string "fix" is cut& pasted from the >> bugzilla title: >> >> http://d.puremagic.com/issues/show_bug.cgi?id=5412 >> >> Easy as pie. Doing it this way means that bugzilla and github automatically >> talk to each other about the fix. It makes life easier for the poor sod >> trying to keep the cross references up to date, give proper credit to the >> submitter, etc. >> >> It is not necessary to invent a new convention for this. Please don't. >> >> >> 2. The pull request often comes with a nice and lucid explanation about the >> fix, oddities about it, etc. But then the actual code patch has no comments >> at all. The code is where the explanation of how the fix works goes. It's >> nice to have it in the pull request too, but please cut&paste it into a >> comment in the right place in the code. If there's a long-winded discussion >> in bugzilla or the n.g. about how things should work, it can be nice to put >> a link to that discussion in the comments. >> >> >> 3. Please do not combine independent fixes into one pull request. Each fix >> should live in its own pull request. The problem with combining them is it's >> a darn nuisance if one of them isn't right. >> >> >> 4. It's nice if, in the pull request message, provide a link to the bugzilla >> entry it addresses. This makes it quick&easy for me to see what's up with >> the bug report. >> >> >> 5. You guys have been great about including test cases that verify the fix. >> Please continue! >> >> _______________________________________________ >> phobos mailing list >> phobos@puremagic.com >> http://lists.puremagic.com/mailman/listinfo/phobos > _______________________________________________ > phobos mailing list > phobos@puremagic.com > http://lists.puremagic.com/mailman/listinfo/phobos > > _______________________________________________ phobos mailing list phobos@puremagic.com http://lists.puremagic.com/mailman/listinfo/phobos |
February 19, 2012 Re: [phobos] Pull request style | ||||
---|---|---|---|---|
| ||||
Posted in reply to Walter Bright | On Sun, Feb 19, 2012 at 16:06, Walter Bright <walter@digitalmars.com> wrote: > > > On 2/18/2012 8:39 PM, Daniel Murphy wrote: >> >> 1. Walter, what about pull requests for D1/D2 bugs? These don't completely fix the bug, they require a D1 patch before it can be closed. I didn't get a reply when I asked this before. (Sometimes I just forget) > > > I generally just fold those into D1 myself. > Yeah, but do you want github autoclosing the bug report when the D2 patch is merged? The bug isn't really fixed until the change is merged into the D1 branch as well. Maybe it would be possible to have the bugzilla issue autoclosed only if it's marked as D2 only, otherwise just remove the D2 tag... Brad, is this something that would be easy/possible to do? I have no idea how customizable the github/bugzilla interaction is. _______________________________________________ phobos mailing list phobos@puremagic.com http://lists.puremagic.com/mailman/listinfo/phobos |
February 18, 2012 Re: [phobos] Pull request style | ||||
---|---|---|---|---|
| ||||
Posted in reply to Daniel Murphy | On 2/18/2012 9:27 PM, Daniel Murphy wrote: > On Sun, Feb 19, 2012 at 16:06, Walter Bright <walter@digitalmars.com> wrote: >> >> >> On 2/18/2012 8:39 PM, Daniel Murphy wrote: >>> >>> 1. Walter, what about pull requests for D1/D2 bugs? These don't completely fix the bug, they require a D1 patch before it can be closed. I didn't get a reply when I asked this before. (Sometimes I just forget) >> >> >> I generally just fold those into D1 myself. >> > > Yeah, but do you want github autoclosing the bug report when the D2 patch is merged? The bug isn't really fixed until the change is merged into the D1 branch as well. Maybe it would be possible to have the bugzilla issue autoclosed only if it's marked as D2 only, otherwise just remove the D2 tag... > > Brad, is this something that would be easy/possible to do? I have no idea how customizable the github/bugzilla interaction is. Currently they're not auto-closed, they're only auto-annotated with the pull requests. The code for doing the annotations and closes is a generic hook run by the guys at github and it has no way of doing any project specific logic. So, no. _______________________________________________ phobos mailing list phobos@puremagic.com http://lists.puremagic.com/mailman/listinfo/phobos |
February 19, 2012 Re: [phobos] Pull request style | ||||
---|---|---|---|---|
| ||||
Posted in reply to Brad Roberts | On Sun, Feb 19, 2012 at 16:46, Brad Roberts <braddr@puremagic.com> wrote: > > Currently they're not auto-closed, they're only auto-annotated with the pull requests. The code for doing the annotations and closes is a generic hook run by the guys at github and it has no way of doing any project specific logic. > > So, no. Damn. I can't wait until we get to ditch D1 at the end of they year. Walter, do you want 'fix' on pulls for D2/D1 bugs? If so I'll go through and add it to all of mine. _______________________________________________ phobos mailing list phobos@puremagic.com http://lists.puremagic.com/mailman/listinfo/phobos |
February 18, 2012 Re: [phobos] Pull request style | ||||
---|---|---|---|---|
| ||||
Posted in reply to Daniel Murphy | On 2/18/2012 9:53 PM, Daniel Murphy wrote: > On Sun, Feb 19, 2012 at 16:46, Brad Roberts<braddr@puremagic.com> wrote: >> Currently they're not auto-closed, they're only auto-annotated with the pull requests. The code for doing the >> annotations and closes is a generic hook run by the guys at github and it has no way of doing any project specific logic. >> >> So, no. > Damn. I can't wait until we get to ditch D1 at the end of they year. > > Walter, do you want 'fix' on pulls for D2/D1 bugs? If so I'll go > through and add it to all of mine. > _______________________________________________ > At the moment it's not *really* necessary, since the hook for 'fix' isn't there yet, so as long as they have the "issue nnnn" they're ok. I'd just like this to be the style going forwards. _______________________________________________ phobos mailing list phobos@puremagic.com http://lists.puremagic.com/mailman/listinfo/phobos |
February 19, 2012 Re: [phobos] Pull request style | ||||
---|---|---|---|---|
| ||||
Posted in reply to Walter Bright | Ok, I'll do so in the future. On Sun, Feb 19, 2012 at 17:28, Walter Bright <walter@digitalmars.com> wrote: > > > On 2/18/2012 9:53 PM, Daniel Murphy wrote: >> >> On Sun, Feb 19, 2012 at 16:46, Brad Roberts<braddr@puremagic.com> wrote: >>> >>> Currently they're not auto-closed, they're only auto-annotated with the >>> pull requests. The code for doing the >>> annotations and closes is a generic hook run by the guys at github and it >>> has no way of doing any project specific logic. >>> >>> So, no. >> >> Damn. I can't wait until we get to ditch D1 at the end of they year. >> >> Walter, do you want 'fix' on pulls for D2/D1 bugs? If so I'll go through and add it to all of mine. _______________________________________________ >> > > At the moment it's not *really* necessary, since the hook for 'fix' isn't there yet, so as long as they have the "issue nnnn" they're ok. I'd just like this to be the style going forwards. > > _______________________________________________ > phobos mailing list > phobos@puremagic.com > http://lists.puremagic.com/mailman/listinfo/phobos _______________________________________________ phobos mailing list phobos@puremagic.com http://lists.puremagic.com/mailman/listinfo/phobos |
Copyright © 1999-2021 by the D Language Foundation