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

> In general, pull request culture is not about code review.

This is not true. There are many projects on GitHub which do extensive code reviews on pull requests. It may not be as nice as Gerrit for the type of project like Go (where you often have many iterations or the diffs are large). But for many other projects the UI that GitHub provides is sufficient (and arguably more efficient than Gerrit).



> where you often have many iterations or the diffs are large). But for many other projects the UI that GitHub provides is sufficient

GitHub is painful for non-trivial reviews. Biggest WTFs:

- No comment threading (or at least collapsing). On a PR with 100 comments[1] it is unlikely that those revisiting the thread need to see (and download, and render...) the first bazillion comments.

- Source "annotations" are lost after a force-push (why not keep around a read-only view of old comments? We have lost some valuable discussions on GH pull requests)

Yes, we try to keep PRs small. But they also need to be meaningful, and sometimes they require (many) more reworks than expected.

[1] https://github.com/neovim/neovim/pull/1820


> GitHub is painful for non-trivial reviews.

That may very well be the case. But note how you said for non-tivial reviews, whereas in the presentation about go they said in general (see the line I quoted in my original comment). And argue that the vast majority of pull requests on GitHub are simple ones which don't need much discussion, so in general the GitHub UI works just fine. I don't have any hard numbers to back up my claim.


> And argue that the vast majority of pull requests on GitHub are simple ones which don't need much discussion, so in general the GitHub UI works just fine.

Right. And what you say validates my exact point: "pull request culture is not about code review." If all you want to do is cast your eye over it and click "merge", it works great. That's not how we work, though.


If you said "GitHub pull requests don't fit our culture" or "The GitHub pull request UI is insufficient for our needs" then it'd accept that. Both are perfectly valid reasons for preferring Gerrit. But placing this blanket statement about the whole "pull request culture" is just wrong.


We've spoken to GitHub about this. They're not happy with how PRs work, either. I stand by my statements.


I hope they are going to do something about it and someday see all the Go development happening on GitHub :)


> Source "annotations" are lost after a force-push (why not keep around a read-only view of old comments? We have lost some valuable discussions on GH pull requests)

Yes, that is incredibly annoying. I discovered that if you add your comments in the "Files changed" tab (which shows the diff of the entire pull request) instead of the "Commits" tab (which shows the diff commit-by-commit), then the comments aren't lost when you force-push.

Just FYI, might make life a bit easier if you're stuck with Github.


Wait. Force pushes? That's a terrible habit to get into; force pushes have decimated more than a few a open source project's repository.

https://news.ycombinator.com/item?id=6713742


What? Pull requests should be on their own branches, if you want to keep one commit per PR but need to edit it you must rebase it and thus force push. It’s a terrible habit if you’re sharing your branch with others, it’s completely normal if not.


Force pushes to the PR fork. We have set receive.denyNonFastForwards=true on the upstream master.


For a code review tool more powerful than GitHub's but with a much better UI than Gerrit, check out https://reviewable.io (disclosure: my project). It integrates smoothly with GitHub and doesn't require setting up your own host.


Agreed. What I like most about GitHub pull requests is that they encourage splitting commits into smaller ones. Gerrit is focused on doing a change in one big commit.


Actually, the Gerrit workflow is pretty agnostic about whether you use one or many commits.

In general, it's poor form to send massive commits anyway. Those poor reviewers!




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

Search: