5 ms·

Pull requests are not conversations

111 points by antirez 16y ago

zimpenfish 16y ago
> Bla bla bla, arguing about usefulness, alternatives, what can be improved and so forth. What do you do when the only response is "use the alternatives" and they just plain don't work the same way?
- antirez 16y ago
  code code code -> maintain your branch. Or if this happens too often and you are truly convinced that this should not be the case: code code code -> open a mailing list -> fork the project. if you are right the original developers will eventually merge everything and there will be again a sigle project handled in a bit less dictatorial way.
  - deleted 16y ago
    [deleted]
jasondavies 16y ago
I think it's very valuable to have a discussion thread interleaved with patches that get better with more and more feedback (that's what github's pull requests look like to me). I understand what you mean about the majority of mailing list discussions not starting with a patch though. On github, it would be nice if you could attach a pull request to an existing issue (last time I tried this it didn't work). Perhaps "pull request" is a bit of a misnomer? Technically speaking it's correct, but if the idea was more that you can interleave patches with a discussion, maybe it wouldn't seem so unpalatable to your way of working. If the discussion doesn't end up merged into "master", just close the issue in question as "wontfix" or similar.
- antirez 16y ago
  what is the gain in splitting the conversation about a project between mailing list and github pull requests? Indeed pull requests are often cathedral in the desert, often without a single comment. To centralize the discussion in the mailing list makes a lot more sense IMHO, and exposes it to a much wider audience.
  - jasondavies 16y ago
    Good point, I don't think conversations should ever be split if possible as this is confusing. I contribute to quite a few projects that only have a github project and associated issue tracker though, and "pull requests as conversations" seem to work fine in those cases. These projects are generally smaller than redis though :-)
- deleted 16y ago
  [deleted]
fizx 16y ago
Oh hey, they finally let you comment on a line in the overall diff. Thanks guys!
- antirez 16y ago
  that's why in the new model I propose you don't attach patches, but links to github topic branches. Github is a great tool used this way. For instance instead of polluting the thread about the design with a stupid thing about: "you could also check this other syscall" that's better served by an inline github comment.
zimbatm 16y ago
At least, pull-requests should be also advertised on the project's mailing-list so that conversation can be kept central.
- 0x44 16y ago
  When you submit a pull request, github sends an email to the addresses you have on record. All that would be required for your suggestion would be to add the project's mailing-list to the list of emails on the account. Though emails would be sent for every other project on your account also.
  - antirez 16y ago
    The problem with this is in turn that perhaps the pull requester is not subscribed at all in the ML...
  - zimbatm 16y ago
    But replies from the mailing-list are not integrated into the pull-request comments, so it's not a 1<=>1 integration.
oomkiller 16y ago
On the other hand, lots of small projects don't have mailing lists, and might not want them. The enhanced pull request feature allows a smaller community to have a good discussion about a new feature, without having to maintain a mailing list. I think the real solution here is for Github to add a feature where pull request notifications can be sent to arbitrary email adresses (though you can probably hack around this today). This way, if the project does have a mailing list, and prefers to discuss pull requests there, then the mailing list is not out of the loop.
- antirez 16y ago
  That's a good point. For small projects probably pull requests are providing a way to provide a patch that is much better than using private emails and so forth.
gokhan 16y ago
I was reading this 10 minutes ago :) The best bug reports are pull requests: http://ayende.com/Blog/archive/2011/01/27/the-best-bug-reports-are-pull-requests.aspx http://ayende.com/Blog/archive/2011/01/27/the-best-bug-repor...
- masklinn 16y ago
  Well, antirez was talking about new features and their discussion, one could assert bugs are not new features and have no requirement for discussion beyond "does this fix the bug?" and "does this code not suck?"
  - gokhan 16y ago
    Although I have seen bugs which resulted in feature changes, I'm not arguing for or against anything. It was just another post around the same subject from an equally hardcore NoSQL hacker.
metabrew 16y ago
Prescient - I read this right after patching [1] jekyll (static site generator) and sending mojombo a pull request on github without any mailing list or other discussion. For quick/simple patches, I find the pull-request approach to be way less hassle. As much as I value the open source process, I simply can't be bothered to join the mailing list, post, wait for a reply etc etc for every project I write a trivial patch for, especially if it's a project I'm not planning to hack on a lot. Life's too short. At least this way, if the project authors want my patch, they can get it easily. Otherwise I'm happy for it to just sit in my fork. Common sense is required - sending a pull-request out of the blue for a large or structural change is asking for trouble. For trivial patches it's very convenient. [1] https://github.com/RJ/jekyll/commit/2f9d4f5be623f37ca79adf84de83fcc3a1c757d6 https://github.com/RJ/jekyll/commit/2f9d4f5be623f37ca79adf84...
- antirez 16y ago
  Completely agree, for small projects, or small patches of big projects, pull requests are not that bad, but neither very good IMHO as there is always a lot of background missing. But the kind of pull requests I receive are often fairly large. New commands implemented or other large changes.
  - dwc 16y ago
    And there's the thing: not all patches are the same. It's not even so much the size of the patch, but rather how much it gets into the design, I think. If someone sends a largish bugfix patch that doesn't touch design then that seems ok to me. Even if you don't like the actual patch, you'll probably do your own similar (but correct) patch. But sending even a smallish patch that messes with design would have been better with discussion first. Even small tweaks to architecture, API/ABI, UX, etc. need careful consideration.
judofyr 16y ago
What I find most annoying is that in GitHub's world, a pull request is an issue (e.g. see here: https://github.com/rtomayko/tilt/issues https://github.com/rtomayko/tilt/issues). That's totally backwards to me. A pull request is a solution to an issue. This workflow is natural to me: 1. Someone reports an issue 2. A patch is provided If you follow this workflow today, you'll end up with two issues in the issue tracker. Confusing! It should be one issue and one fix for that issue. And if someone sends a pull request (without opening a ticket first), what should I do if I agree with the feature/bugfix, but not with the way it was solved? The issue isn't fixed yet, so it's wrong to close it, but it feels weird to keep the pull request open (since I've actually rejected it). I want to close the pull request only!
- rubiii 16y ago
  my thoughts exactly!
ekidd 16y ago
As an open source author, I prefer to discuss features with people after they send their first patch. GitHub allows me to sort potential contributors into 3 categories: 1) Programmers who talk a lot without sending patches will be gently discouraged by the lack of a forum. 2) Programmers who send dirty patches, or who don't write unit tests, can be given some quick mentoring and asked to please resubmit. 3) Programmers who send clean patches with unit tests and well-written commit messages can be welcomed with open arms and fulsome praise. If you're a UI/UX expert, or a designer, then please substitute "blog post with pictures" for "patch". All types of skills are valuable.
- antirez 16y ago
  > As an open source author, I prefer to discuss features with people after they send their first patch. Sure for the developer that is easy, but I don't feel well when I think at all the wasted work. Imagine running a company this way... Or compare the two models in this very real world scenario: "that's my patch, looks good?", "no sorry it was already a work in progress and is almost finished". With instead "what about adding this feature?", "it's already in the workings, thanks, but we could like to have that if you like to contribute". But perhaps your open source project has a different nature compared to Redis. Being Redis composed of commands and data types, it is very similar to a programming language. And if you think about evolving a language in this way, via pull requests, it is easy to realize how this can't work. It's a lot more about design.
  - rmetzler 16y ago
    I guess it wouldn't harm to state the patching policy inside the README file.
    - antirez 16y ago
      In Redis we have a "CONTRIBUTING" file in 2.2 and unstable where this is stated very clearly.
      - 16y ago
dgit 16y ago
I use git-backed wikis for the 'conversation' part. Project mailing lists are wrong for the same reason that Subversion/CVS were wrong: Centralization turns a project political, at least that's my experience. Git's distributed storage encourages forking at a whim. If your 'conversation' is via a wiki you can fork that conversation any second. Git-backed wiki how-to: My "wikis" currently simply are HTML pages served by gitweb right out of the repo, however I am experimenting with gollum and smeagol. That is: if your repos are on some server of your own, not github. I have no exprience with github and won't be caught dead using it.
- antirez 16y ago
  There is nothing worse than collaborative design in this world. To extend the article example, this is why architectural masterpieces are more or less never the result of collaborative design. Why designing langauges, databases, user interfaces, and in general programs should be different? It is not.
- metageek 16y ago
  >I have no exprience with github and won't be caught dead using it. ...because?
  - dgit 16y ago
    - Should it be out of service for a short time, it'd be disrupting my workflow. - Should it be out of service for a long time, I'd have to move to a different server and risk losing contacts that only know me by my github name/URL. - Most importantly: On my server I have repos that are more open that 'private' but less open than 'public'. E.g. I hand out invites to a repo to attendees of a meeting. Or I limit by location using geo-IP.
dmethvin 16y ago
I'm on the jQuery team and a pull request should definitely not be the first step for us, especially for a first-time contributor. An over-the-transom solution could use an inappropriate approach, may be too much code to justify adding the feature to the project, may not meet project coding standards, or could be a duplicate because there is already a different patch landed by someone else in a branch somewhere. Pull requests are also not bug tickets. We'd prefer that you ask on the forum if you'd like to discuss feature changes, or file a ticket (http://docs.jquery.com/How_to_Report_Bugs http://docs.jquery.com/How_to_Report_Bugs) in the bug tracker. GitHub's pull queue doesn't provide a lot of control, which makes it less than ideal as a first-contact mechanism for a busy project. Requests can only be opened, closed, and sorted by three relatively weak criteria. That's one reason why we prefer to use the bug tracker for managing things, we have a lot more options for reporting and prioritizing there.
collypops 16y ago
This is what happens when you host new and exciting open source projects on GitHub, a social coding website. Contributing to open source projects is so much more accessible than it used to be, and the exposure that GitHub gives these projects will inevitably attract all kinds of people who all have various ideas about the direction the project should be taken. Before GitHub, these sorts of projects were announced on IRC, and were then discussed on IRC, so that everyone participating had a better idea about what direction the core developers wanted the project to be taken in. This made it more likely that pull requests were for features/bugs that the project team actively wanted people to work on. GitHub is missing this kind of environment, so it's almost inevitable that first contact between maintainers and potential contributors usually comes in the form of a pull request.
davidw 16y ago
> the project mailing list is a market for the project design It's also where design decisions are recorded so that people can come along and read about them in the future. Excellent article: it points out one of the things I don't like so much about people's use of github, in a constructive way.
famousactress 16y ago
Really good points, though I see benefit in the existing pull request mechanism also. It'd be interesting to consider merging the concepts of opening tickets and making pull requests.. For instance, if I could 'pull request' a conceptual feature, before there's code written and then we could discuss, add patches, and eventually merge a solution. It'd allow each project and issue the opportunity to do what's appropriate for the particular change.
deleted 16y ago
[deleted]
jesstaa 16y ago
The Go project has a pretty good way of doing this. http://golang.org/doc/contribute.html http://golang.org/doc/contribute.html Posting a changelist for review also posts to the dev mailing list for discussion and approval.

111 points by antirez 16y ago

> Bla bla bla, arguing about usefulness, alternatives, what can be improved and so forth. What do you do when the only response is "use the alternatives" and they just plain don't work the same way?

antirez 16y ago
code code code -> maintain your branch. Or if this happens too often and you are truly convinced that this should not be the case: code code code -> open a mailing list -> fork the project. if you are right the original developers will eventually merge everything and there will be again a sigle project handled in a bit less dictatorial way.
- deleted 16y ago
  [deleted]

jasondavies 16y ago

I think it's very valuable to have a discussion thread interleaved with patches that get better with more and more feedback (that's what github's pull requests look like to me). I understand what you mean about the majority of mailing list discussions not starting with a patch though. On github, it would be nice if you could attach a pull request to an existing issue (last time I tried this it didn't work). Perhaps "pull request" is a bit of a misnomer? Technically speaking it's correct, but if the idea was more that you can interleave patches with a discussion, maybe it wouldn't seem so unpalatable to your way of working. If the discussion doesn't end up merged into "master", just close the issue in question as "wontfix" or similar.

antirez 16y ago
what is the gain in splitting the conversation about a project between mailing list and github pull requests? Indeed pull requests are often cathedral in the desert, often without a single comment. To centralize the discussion in the mailing list makes a lot more sense IMHO, and exposes it to a much wider audience.
- jasondavies 16y ago
  Good point, I don't think conversations should ever be split if possible as this is confusing. I contribute to quite a few projects that only have a github project and associated issue tracker though, and "pull requests as conversations" seem to work fine in those cases. These projects are generally smaller than redis though :-)
deleted 16y ago
[deleted]

fizx 16y ago

Oh hey, they finally let you comment on a line in the overall diff. Thanks guys!

antirez 16y ago
that's why in the new model I propose you don't attach patches, but links to github topic branches. Github is a great tool used this way. For instance instead of polluting the thread about the design with a stupid thing about: "you could also check this other syscall" that's better served by an inline github comment.

zimbatm 16y ago

At least, pull-requests should be also advertised on the project's mailing-list so that conversation can be kept central.

0x44 16y ago
When you submit a pull request, github sends an email to the addresses you have on record. All that would be required for your suggestion would be to add the project's mailing-list to the list of emails on the account. Though emails would be sent for every other project on your account also.
- antirez 16y ago
  The problem with this is in turn that perhaps the pull requester is not subscribed at all in the ML...
- zimbatm 16y ago
  But replies from the mailing-list are not integrated into the pull-request comments, so it's not a 1<=>1 integration.

oomkiller 16y ago

On the other hand, lots of small projects don't have mailing lists, and might not want them. The enhanced pull request feature allows a smaller community to have a good discussion about a new feature, without having to maintain a mailing list. I think the real solution here is for Github to add a feature where pull request notifications can be sent to arbitrary email adresses (though you can probably hack around this today). This way, if the project does have a mailing list, and prefers to discuss pull requests there, then the mailing list is not out of the loop.

antirez 16y ago
That's a good point. For small projects probably pull requests are providing a way to provide a patch that is much better than using private emails and so forth.

gokhan 16y ago

I was reading this 10 minutes ago :) The best bug reports are pull requests: http://ayende.com/Blog/archive/2011/01/27/the-best-bug-reports-are-pull-requests.aspx http://ayende.com/Blog/archive/2011/01/27/the-best-bug-repor...

masklinn 16y ago
Well, antirez was talking about new features and their discussion, one could assert bugs are not new features and have no requirement for discussion beyond "does this fix the bug?" and "does this code not suck?"
- gokhan 16y ago
  Although I have seen bugs which resulted in feature changes, I'm not arguing for or against anything. It was just another post around the same subject from an equally hardcore NoSQL hacker.

metabrew 16y ago

Prescient - I read this right after patching [1] jekyll (static site generator) and sending mojombo a pull request on github without any mailing list or other discussion. For quick/simple patches, I find the pull-request approach to be way less hassle. As much as I value the open source process, I simply can't be bothered to join the mailing list, post, wait for a reply etc etc for every project I write a trivial patch for, especially if it's a project I'm not planning to hack on a lot. Life's too short. At least this way, if the project authors want my patch, they can get it easily. Otherwise I'm happy for it to just sit in my fork. Common sense is required - sending a pull-request out of the blue for a large or structural change is asking for trouble. For trivial patches it's very convenient. [1] https://github.com/RJ/jekyll/commit/2f9d4f5be623f37ca79adf84de83fcc3a1c757d6 https://github.com/RJ/jekyll/commit/2f9d4f5be623f37ca79adf84...

antirez 16y ago
Completely agree, for small projects, or small patches of big projects, pull requests are not that bad, but neither very good IMHO as there is always a lot of background missing. But the kind of pull requests I receive are often fairly large. New commands implemented or other large changes.
- dwc 16y ago
  And there's the thing: not all patches are the same. It's not even so much the size of the patch, but rather how much it gets into the design, I think. If someone sends a largish bugfix patch that doesn't touch design then that seems ok to me. Even if you don't like the actual patch, you'll probably do your own similar (but correct) patch. But sending even a smallish patch that messes with design would have been better with discussion first. Even small tweaks to architecture, API/ABI, UX, etc. need careful consideration.

judofyr 16y ago

What I find most annoying is that in GitHub's world, a pull request is an issue (e.g. see here: https://github.com/rtomayko/tilt/issues https://github.com/rtomayko/tilt/issues). That's totally backwards to me. A pull request is a solution to an issue. This workflow is natural to me: 1. Someone reports an issue 2. A patch is provided If you follow this workflow today, you'll end up with two issues in the issue tracker. Confusing! It should be one issue and one fix for that issue. And if someone sends a pull request (without opening a ticket first), what should I do if I agree with the feature/bugfix, but not with the way it was solved? The issue isn't fixed yet, so it's wrong to close it, but it feels weird to keep the pull request open (since I've actually rejected it). I want to close the pull request only!

rubiii 16y ago
my thoughts exactly!

ekidd 16y ago

As an open source author, I prefer to discuss features with people after they send their first patch. GitHub allows me to sort potential contributors into 3 categories: 1) Programmers who talk a lot without sending patches will be gently discouraged by the lack of a forum. 2) Programmers who send dirty patches, or who don't write unit tests, can be given some quick mentoring and asked to please resubmit. 3) Programmers who send clean patches with unit tests and well-written commit messages can be welcomed with open arms and fulsome praise. If you're a UI/UX expert, or a designer, then please substitute "blog post with pictures" for "patch". All types of skills are valuable.

antirez 16y ago
> As an open source author, I prefer to discuss features with people after they send their first patch. Sure for the developer that is easy, but I don't feel well when I think at all the wasted work. Imagine running a company this way... Or compare the two models in this very real world scenario: "that's my patch, looks good?", "no sorry it was already a work in progress and is almost finished". With instead "what about adding this feature?", "it's already in the workings, thanks, but we could like to have that if you like to contribute". But perhaps your open source project has a different nature compared to Redis. Being Redis composed of commands and data types, it is very similar to a programming language. And if you think about evolving a language in this way, via pull requests, it is easy to realize how this can't work. It's a lot more about design.
- rmetzler 16y ago
  I guess it wouldn't harm to state the patching policy inside the README file.
  - antirez 16y ago
    In Redis we have a "CONTRIBUTING" file in 2.2 and unstable where this is stated very clearly.
    - 16y ago

dgit 16y ago

I use git-backed wikis for the 'conversation' part. Project mailing lists are wrong for the same reason that Subversion/CVS were wrong: Centralization turns a project political, at least that's my experience. Git's distributed storage encourages forking at a whim. If your 'conversation' is via a wiki you can fork that conversation any second. Git-backed wiki how-to: My "wikis" currently simply are HTML pages served by gitweb right out of the repo, however I am experimenting with gollum and smeagol. That is: if your repos are on some server of your own, not github. I have no exprience with github and won't be caught dead using it.

antirez 16y ago
There is nothing worse than collaborative design in this world. To extend the article example, this is why architectural masterpieces are more or less never the result of collaborative design. Why designing langauges, databases, user interfaces, and in general programs should be different? It is not.
metageek 16y ago
>I have no exprience with github and won't be caught dead using it. ...because?
- dgit 16y ago
  - Should it be out of service for a short time, it'd be disrupting my workflow. - Should it be out of service for a long time, I'd have to move to a different server and risk losing contacts that only know me by my github name/URL. - Most importantly: On my server I have repos that are more open that 'private' but less open than 'public'. E.g. I hand out invites to a repo to attendees of a meeting. Or I limit by location using geo-IP.

dmethvin 16y ago

I'm on the jQuery team and a pull request should definitely not be the first step for us, especially for a first-time contributor. An over-the-transom solution could use an inappropriate approach, may be too much code to justify adding the feature to the project, may not meet project coding standards, or could be a duplicate because there is already a different patch landed by someone else in a branch somewhere. Pull requests are also not bug tickets. We'd prefer that you ask on the forum if you'd like to discuss feature changes, or file a ticket (http://docs.jquery.com/How_to_Report_Bugs http://docs.jquery.com/How_to_Report_Bugs) in the bug tracker. GitHub's pull queue doesn't provide a lot of control, which makes it less than ideal as a first-contact mechanism for a busy project. Requests can only be opened, closed, and sorted by three relatively weak criteria. That's one reason why we prefer to use the bug tracker for managing things, we have a lot more options for reporting and prioritizing there.

collypops 16y ago

This is what happens when you host new and exciting open source projects on GitHub, a social coding website. Contributing to open source projects is so much more accessible than it used to be, and the exposure that GitHub gives these projects will inevitably attract all kinds of people who all have various ideas about the direction the project should be taken. Before GitHub, these sorts of projects were announced on IRC, and were then discussed on IRC, so that everyone participating had a better idea about what direction the core developers wanted the project to be taken in. This made it more likely that pull requests were for features/bugs that the project team actively wanted people to work on. GitHub is missing this kind of environment, so it's almost inevitable that first contact between maintainers and potential contributors usually comes in the form of a pull request.

davidw 16y ago

> the project mailing list is a market for the project design It's also where design decisions are recorded so that people can come along and read about them in the future. Excellent article: it points out one of the things I don't like so much about people's use of github, in a constructive way.

famousactress 16y ago

Really good points, though I see benefit in the existing pull request mechanism also. It'd be interesting to consider merging the concepts of opening tickets and making pull requests.. For instance, if I could 'pull request' a conceptual feature, before there's code written and then we could discuss, add patches, and eventually merge a solution. It'd allow each project and issue the opportunity to do what's appropriate for the particular change.

deleted 16y ago

[deleted]

jesstaa 16y ago

The Go project has a pretty good way of doing this. http://golang.org/doc/contribute.html http://golang.org/doc/contribute.html Posting a changelist for review also posts to the dev mailing list for discussion and approval.