Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

It's easy for the original author to make changes to the PR, but not easy for the maintainer or a third party to make changes. The micro-pedantry associated with spelling errors, tweaking commit messages, and trivial spacing/formatting issues can be more work for everyone when communicated through comments on PRs with the original author expected to apply and re-roll.


That's only partially true.

Pull requests on GitHub have to have an associated branch or fork in another repo. Even if you're not the original author, if you have access to it, you can do "git checkout" or "git clone", then you make the required changes and then on push the commits are automatically included in the pull request.

The only problem is that the maintainers do not have write access to the forked repositories of new or casual contributors. So if you're in a rush to change something, you have to fork that branch, make changes and then merge it, while ignoring the original pull request (although if you place the issue number in the commit messages you push, they'll get referenced on the associated page).

However, the commits logs made by the wannabe contributor are still in the history, so proper credit/blame is given where it is due, unless you rebase of course. And this isn't something you can say about diffs sent by email.

Also, GitHub sends you emails on pull requests. And you can reply by email too.

What I like about GitHub Pull requests is that you have a link you can give to people for review or that you can post wherever, a link that gives you a descriptive diff of what happened. Plus, the discussions around that pull request are left there, included in that link, left for everybody to see, instead of turning into a private conversation that nobody else will know about (incidentally, it's also the reason why I think native apps will never beat web apps, as long as native apps don't have URLs for referring to content or state within the app). This to me is useful enough to prefer it over diffs by email.

EDIT: there is one thing about GitHub I don't like - you cannot give write-access to people, while preventing them from pushing to master. If this where possible, you could give everybody access to push, except for master, in which case the above mentioned issue with pull requests would be a non-issue!


The problems with keeping the original commit and then putting the fixes on top are, (a) it makes the history harder to follow, (b) it makes it easy for someone to screw up a cherry-pick of a bug fix to a maintenance brance, since now you have to cherry-pick the bug fix plus the fix(es) to the bug fix, and (c) if the original patch had a potential bug that might cause the system to crash or otherwise malfunction, even if you fix it in the subsequent commit, it makes "git bisects" more likely to yield false positives, or at least make things more confusing and more difficult than it has to be.


Even when sending patches via email, I've always been asked by maintainers to do the trivialities myself and re-submit. I suspect at least part of the reason was education.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: