Pull requests are not conversations | Hacker News Reader

Pull requests are not conversations

antirez.com

111 pointsantirez47 comments

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/2f9d4f5be623f37ca79adf84...

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.

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.

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). 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!

my thoughts exactly!

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.

> 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.

I guess it wouldn't harm to state the patching policy inside the README file.

In Redis we have a "CONTRIBUTING" file in 2.2 and unstable where this is stated very clearly.

+1 Adding a license and contribution policies to an open source project are great preconditions to get bugfixes and feature contributions.

But I wonder why then sending you pull requests is an issue?

Perhaps I was overly harsh. :-) Different projects have different needs.

Some projects work best with a small number of designers and core contributers, and a larger number of people who submit an occasional bug fix (or good bug reports). In a project like that, bringing a perspective contributor on board can take several hours of my time. If I tried to invest that time for everybody who said, "Hey, I'd like to implement feature X" or who sent a dodgy patch, I'd never make any forward progress.

So in self-defense, I now encourage people to start by contributing a small, simple patch. That gives me a good idea of how much time I should invest on bringing them up to speed.

Imagine running a company this way...

Essentially, a potential contributor's first patch is a job interview. Once they demonstrate that they can help the project, then it's worth spending a lot of time with them. But even in interviews, I want to see code.

I think it's a bit paternalistic to be concerned about this because of "wasted work". One of the best ways to learn about a piece of code is to start hacking on it, regardless of whether or not any changes you make or features you add will be needed or wanted by the upstream project.

If someone does the work unsolicited and then complains that you're not merging it, that's a problem, sure. But otherwise, you shouldn't feel bad about what other people choose to do with their own time.

Hmm, "the sum of all wasted work by proven developers dealing with people who have no intention of following through" vs. "the sum of all work wasted by people who follow through to some extent and don't get their patches accepted for some reason"... honestly, it's a tough call which is larger even on a straight-across time comparison, even before we account for the fact that the first group of people are getting more productivity per hour.

As a programmer who has way more things to do than I have time for, no way am I just going to go off and code a patch for your project without first finding out if you'd be receptive to it.

I'd feel like I wasted my time if I sent you a patch that was clean, had tests, and otherwise met all your requirements, but you rejected it because my proposed feature doesn't fit in with your philosophy of how the program should work, or if it turned out you were already coding that feature and your code was far enough along that you preferred to stick with it than switch to mine.

I think Open Source development works a lot better where people implement features/fixes that they themselves want/need, at least for the first few patches. This way, you haven't wasted your time by writing the code to scratch your itch because, even if it is rejected, you can still patch your own version and have the feature you want.

> even if it is rejected, you can still patch your own version and have the feature you want

One longer term issue there is that you are now stuck maintaining this code through any changes the upstream authors choose to make in the future (if you really want this feature that bad)

As a programmer who has way more things to do than I have time for...

I think this is a perfectly fair response. Good patches take me about 10 seconds to merge, and bad patches take me 20+ minutes, because I have to (1) understand what they're supposed to accomplish, (2) write unit tests, and (3) rewrite the code. In a case like that, it's usually a waste of everybody's time.

In general, when I start contributing to a new project, I begin with a bug fix, and include both unit tests and a detailed commit message. Then I plan on rewriting it at least once, because the upstream project has perfectly valid constraints that I don't understand.

To be fair, I've submitted a variety of pull requests that have been ignored... no comments, no response, etc. At least with the pull request format, if the author is too busy to maintain the project he/she could theoretically just accept requests and spend about 2 minutes a month being maintainer.

'fulsome' means the opposite of what you think it means.

"Fulsome" means "abundant". I think that was correct, from the context. The fact that it is often used sarcastically only means that you need to determine from context whether it's sincere or insincere. I think it was clear, in this context.

http://en.wiktionary.org/wiki/fulsome

That's not the most common usage listed; the most common, which also happens to match the fixed phrase you used, is "offensive to good taste", the second most common is "disgusting, sickening, repulsive", the third most common is "insincere", cf. http://dictionary.reference.com/browse/fulsome

1) The phrase that _I_ used? When did I use it?

2) Okay, fine, we'll use YOUR reference instead of MINE, even though they basically say the same thing. What they say is that there are multiple meanings, and some of those meanings are the opposite of one another. In situations like that, the reader gets to work it out. It's not a case of having simply used the word wrong, as you claimed. In fact, both dictionaries (wikt, dictionary.com) specifically mention this in their usage notes. So why are you citing a source that agrees with me?

Scroll down in the link YOU chose, to where Collins Dictionary senses 2 and 3 are consistent with the OP's intent.

Scroll down in the link YOU chose, to where the Online Etymology Dictionary says that for 50 years now it is coming back into fashion to use it in the original sense of "abundant".

If you're a descriptivist, then the fact is that people use it this way. And if you're a prescriptivist who prefers original meanings, then use the original (15c) meaning. Prescriptivists who only know about 100 years of history are only fooling themselves.

Don't get me wrong, I'm all for pedantic corrections, it's a bad habit of mine too. But only when they're correct. You said "ekidd, you're using this word wrong", but actually, ekidd's usage was perfectly valid, as even your source of choice admits.

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) 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.

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-repor...

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?"

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.

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.

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.

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.

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.

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.

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 :-)

> 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.

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.

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.

>I have no exprience with github and won't be caught dead using it.

...because?

- 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.

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.

The Go project has a pretty good way of doing this. http://golang.org/doc/contribute.html

Posting a changelist for review also posts to the dev mailing list for discussion and approval.

> 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?

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.

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

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.

The problem with this is in turn that perhaps the pull requester is not subscribed at all in the ML...

But replies from the mailing-list are not integrated into the pull-request comments, so it's not a 1<=>1 integration.

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

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.

Reply on news.ycombinator.com