Hi folks,
Don't throw any tomatoes just yet! We've been going back and forth about using pull requests on GitHub for a long time. I'd like to propose that we slowly try an experiment. We've had a few outside contributions come in through pull requests, and it's worked just fine. At the same time, there are people who are more comfortable with sending patches to the list. So, even though it seems like it would be nice to have everyone work the same way, it seems like the two options aren't mutually exclusive.
I'm proposing that we allow people who want to use pull requests to try it out. Send pull requests, and other people who want to try pull requests can review them. If you don't like pull requests, don't use them, and someone will review and push like we do today. If you're on the fence, you can try it out, or not.
I don't love that this would cause us to split patch discussion across two places, but I think it's worth it as an experiment. I think it will help us iron out any kinks, and experiment with whether this is something we want to go forward with. I don't think forcing everyone to switch to pull requests (especially overnight) is a good idea, nor is ignoring the subject because it's controversial. This gets things moving, and I hope it will be like our Fedora vs. upstream gem thing, where trying to support both ends up making us more robust, versus needlessly distracting us.
I'd like to give this a try. Would anyone care to join me?
-- Matt
On 08/06/2012 10:59 PM, Matt Wagner wrote:
Hi folks,
Don't throw any tomatoes just yet! We've been going back and forth about using pull requests on GitHub for a long time. I'd like to propose that we slowly try an experiment. We've had a few outside contributions come in through pull requests, and it's worked just fine. At the same time, there are people who are more comfortable with sending patches to the list. So, even though it seems like it would be nice to have everyone work the same way, it seems like the two options aren't mutually exclusive.
I'm proposing that we allow people who want to use pull requests to try it out. Send pull requests, and other people who want to try pull requests can review them. If you don't like pull requests, don't use them, and someone will review and push like we do today. If you're on the fence, you can try it out, or not.
I don't love that this would cause us to split patch discussion across two places, but I think it's worth it as an experiment. I think it will help us iron out any kinks, and experiment with whether this is something we want to go forward with. I don't think forcing everyone to switch to pull requests (especially overnight) is a good idea, nor is ignoring the subject because it's controversial. This gets things moving, and I hope it will be like our Fedora vs. upstream gem thing, where trying to support both ends up making us more robust, versus needlessly distracting us.
I'd like to give this a try. Would anyone care to join me?
-- Matt
It's probably worth noting that at least three Aeolus-related projects do use the GitHub pull-request model: Image Factory[1], Oz[2] and Image Management Engine[3].
I like the proposition. Forcing everyone over without proper evaluation would be silly but I do think that sending patches to the list is far from perfect solution.
Especially for new contributors who are not used to that form of collaboration.
While we're at it, what about proper git (as oposed to GitHub) pull requests? That is, you have a publicly readable repo (on GitHub or elsewhere) and when you want someone to review and merge your changes you send them the link to the repo and the branch name to pull.
If I remember correctly, that was the modus operandi for which git was invented.
It provides a nice balance between splitting the conversation about the patches (github and mailing list) whilst still being reasonably simple to use (it adds about one additional step on each side compared to GitHub pull requests).
Thomas
[1]: https://github.com/aeolusproject/imagefactory [2]: https://github.com/clalancette/oz [3]: https://github.com/aeolus-incubator/image-management-engine
On Tue, Aug 07, 2012 at 01:28:26PM +0200, Tomas Sedovic wrote:
It's probably worth noting that at least three Aeolus-related projects do use the GitHub pull-request model: Image Factory[1], Oz[2] and Image Management Engine[3].
My understanding is that the Incubator projects use them as well.
While we're at it, what about proper git (as oposed to GitHub) pull requests? That is, you have a publicly readable repo (on GitHub or elsewhere) and when you want someone to review and merge your changes you send them the link to the repo and the branch name to pull.
If I remember correctly, that was the modus operandi for which git was invented.
It provides a nice balance between splitting the conversation about the patches (github and mailing list) whilst still being reasonably simple to use (it adds about one additional step on each side compared to GitHub pull requests).
I think you're right about that being a common/intended workflow. But honestly, one of the main things I liked about doing GitHub pull requests is that you get a nice web UI listing all of the outstanding ones.
What made me really want to start using pull requests was the realization that there are a ton of patches outstanding, but that I have no idea what they are, or if someone else is already reviewing them.
-- Matt
On 08/06/2012 10:59 PM, Matt Wagner wrote:
Hi folks,
Don't throw any tomatoes just yet! We've been going back and forth about using pull requests on GitHub for a long time. I'd like to propose that we slowly try an experiment. We've had a few outside contributions come in through pull requests, and it's worked just fine. At the same time, there are people who are more comfortable with sending patches to the list. So, even though it seems like it would be nice to have everyone work the same way, it seems like the two options aren't mutually exclusive.
I'm proposing that we allow people who want to use pull requests to try it out. Send pull requests, and other people who want to try pull requests can review them. If you don't like pull requests, don't use them, and someone will review and push like we do today. If you're on the fence, you can try it out, or not.
I don't love that this would cause us to split patch discussion across two places, but I think it's worth it as an experiment. I think it will help us iron out any kinks, and experiment with whether this is something we want to go forward with. I don't think forcing everyone to switch to pull requests (especially overnight) is a good idea, nor is ignoring the subject because it's controversial. This gets things moving, and I hope it will be like our Fedora vs. upstream gem thing, where trying to support both ends up making us more robust, versus needlessly distracting us.
I'd like to give this a try. Would anyone care to join me?
-- Matt
I'm with you. I've been wanting to try this for a while now, but my intentions were always a bit too... grandiose and forceful :)
This strikes a good balance and lets those who wish to experiment to do so.
On 07/08/12 09:05 -0400, John Eckersberg wrote:
On 08/06/2012 10:59 PM, Matt Wagner wrote:
Hi folks,
Don't throw any tomatoes just yet! We've been going back and forth about using pull requests on GitHub for a long time. I'd like to propose that we slowly try an experiment. We've had a few outside contributions come in through pull requests, and it's worked just fine. At the same time, there are people who are more comfortable with sending patches to the list. So, even though it seems like it would be nice to have everyone work the same way, it seems like the two options aren't mutually exclusive.
I'm proposing that we allow people who want to use pull requests to try it out. Send pull requests, and other people who want to try pull requests can review them. If you don't like pull requests, don't use them, and someone will review and push like we do today. If you're on the fence, you can try it out, or not.
I don't love that this would cause us to split patch discussion across two places, but I think it's worth it as an experiment. I think it will help us iron out any kinks, and experiment with whether this is something we want to go forward with. I don't think forcing everyone to switch to pull requests (especially overnight) is a good idea, nor is ignoring the subject because it's controversial. This gets things moving, and I hope it will be like our Fedora vs. upstream gem thing, where trying to support both ends up making us more robust, versus needlessly distracting us.
I'd like to give this a try. Would anyone care to join me?
-- Matt
I'm with you. I've been wanting to try this for a while now, but my intentions were always a bit too... grandiose and forceful :)
This strikes a good balance and lets those who wish to experiment to do so.
I am a cautious +1 on this as well. I still have some reservations (like the crappy comment threading/notifications on github), but this is almost exactly what I was trying to propose on irc a little while back (albeit with less positive feedback, probably because I am mean). I personally prefer to have the patch review on a mailing list, as I think it is easier to see context, and has generally higher visibility, but I realize at the same time that a lot of others love the clicky ui of github, and it would be a shame to exclude potential new contributors because we didn't accept pull requests. Lastly, I think if we do this, anyone who is using them should assign themselves (not to their own, to be clear, had trouble wording this) or another team member as the reviewer to make it obvious if a patch if being looked at. This is actually the primary useful point of pull requests to me, so I hope it will be used. We have long complained that it was hard to know if patches were under review, but that will not be the case if we assign reviewers this way.
PS - yes, I know there is a gem for doing cli interaction with github, and I will be trying it, but the setup for reviewing patches doesn't seem great to me.
-j
On Tue, Aug 07, 2012 at 09:25:21AM -0400, Jason Guiditta wrote:
I am a cautious +1 on this as well. I still have some reservations (like the crappy comment threading/notifications on github), but this is almost exactly what I was trying to propose on irc a little while back (albeit with less positive feedback, probably because I am mean).
Yeah, I was actually playing with the inline commenting the other day, and it's not really ideal. It's passable, and will probably work fine if you just had a note or two to mention inline.
I personally prefer to have the patch review on a mailing list, as I think it is easier to see context, and has generally higher visibility, but I realize at the same time that a lot of others love the clicky ui of github, and it would be a shame to exclude potential new contributors because we didn't accept pull requests. Lastly, I think if we do this, anyone who is using them should assign themselves (not to their own, to be clear, had trouble wording this) or another team member as the reviewer to make it obvious if a patch if being looked at. This is actually the primary useful point of pull requests to me, so I hope it will be used. We have long complained that it was hard to know if patches were under review, but that will not be the case if we assign reviewers this way.
Yes, strong +1 to this. The ability to see a list of patches and know which ones are under review is one of the primary reasons I wanted to explore this.
PS - yes, I know there is a gem for doing cli interaction with github, and I will be trying it, but the setup for reviewing patches doesn't seem great to me.
I had the devil of a time getting it to work at all. It wants an API token, which GitHub doesn't do anymore now that their app only does OAuth. I might just need to find a newer version, but I didn't have the energy to extensively debug it the other day. But I've heard lots of people rave about it, which has me thinking that it must just be me doing something wrong.
-- Matt
aeolus-devel@lists.fedorahosted.org