TE
TechEcho
Home24h TopNewestBestAskShowJobs
GitHubTwitter
Home

TechEcho

A tech news platform built with Next.js, providing global tech news and discussions.

GitHubTwitter

Home

HomeNewestBestAskShowJobs

Resources

HackerNews APIOriginal HackerNewsNext.js

© 2025 TechEcho. All rights reserved.

On Pull Requests

58 pointsby cribwiabout 11 years ago

6 comments

depollabout 11 years ago
As far as code reviews go, this is pretty spot on. I was part of a a startup that was using GitHub pull requests for code reviews. As the team grew, it became more and more intractable, although not simply because of notifications. Side-by-side diffs and checkpointed diffs (so that you can see what changed since the last round of review and whether&#x2F;how your comments were addressed) are handled very poorly by GitHub. We ultimately switched to Phabricator, and while there was a little friction as folks got acquainted with the new tool, it made code reviews a <i>much</i> more pleasant process.<p>Recently, I had to go through a full code review back on GH pull requests, and it felt like pulling teeth in comparison. They&#x27;re fine for interacting with contributors to an open source project, but compared to working with a tool like Phabricator that&#x27;s built for a code reviewer&#x27;s workflow (and for teams of engineers working together on a project), they just don&#x27;t hold a candle, in my opinion.
评论 #7697831 未加载
评论 #7698373 未加载
评论 #7698950 未加载
erikbabout 11 years ago
TL;DR Email filtering + git (the shell tool) + text editor solves all of these problems without using fancy tools, imho.<p>To me this looks like a typical attempt to solve a work process problem via technology. Using any code review tool doesn&#x27;t help if you don&#x27;t get code review done right. If you filter notifications based on rules in the tool or let the developers do it via email filtering is basically a question of philosophy. If you look at a Diff that way or another also doesn&#x27;t change much about the review process in itself. Don&#x27;t solve process questions with more software features!<p>IMHO getting loads of emails is great. I can filter by myself what is interesting to me right now. And worse than the one or two uninteresting emails every quarter is when I don&#x27;t get the information I need. A non-engineer middle manager is allowed to complain about email overload in my eyes. A developer should be able to handle pure text.<p>And while we are at it, I also believe that less is more. No matter which repository hosts my code, I do code reviews on my computer with my own developer tools. I&#x27;m not even sure how the current version of GitHub or Bitbucket diffs looks like. These websites are basically the shell scripts and disk space that are used to backup my code and allow other people to access it. If a developer wants to do a code review he can also decide on his tools. The rest is just email and &quot;git merge&quot; (--no-ff). But that&#x27;s just my opinion.
评论 #7698793 未加载
schaconabout 11 years ago
I may be amazingly biased given that I work for GitHub, but a lot of these issues are solved by using the Notifications section of GitHub (<a href="https://github.com/notifications" rel="nofollow">https:&#x2F;&#x2F;github.com&#x2F;notifications</a>).<p>Of the hundreds of repositories we have going internally, the largest is our main GitHub codebase. We have between 80 and 100 developers committing to that one repository every week. This includes something like 160 commits per day across something like 30 pull requests, every day. A lot of us have switched to using the Notifications tab over email (though some use email filtering and others use both). That tab groups all your notifications by project and you can both ack and mute threads easily.<p>I tend to go to Notifications once a day or so and manage everything - mute the stuff I don&#x27;t need to hear from again, unwatch repos where I don&#x27;t need every notification, see where I&#x27;m mentioned, etc. The user and team mentions that this author refers to as a &quot;hack&quot; are in fact a very powerful notification feature. If we need someone from a specific team to review something, we mention them or their team. I find it much more powerful and flexible than the more rigid assignment system of other tools, though you can also assign people to the issues associated with a PR in GitHub as well.<p>The Issues guide on our Guides site is a great overview of these systems and how to use them well for even large teams.<p><a href="https://guides.github.com/features/issues/" rel="nofollow">https:&#x2F;&#x2F;guides.github.com&#x2F;features&#x2F;issues&#x2F;</a><p><a href="https://guides.github.com/introduction/flow/" rel="nofollow">https:&#x2F;&#x2F;guides.github.com&#x2F;introduction&#x2F;flow&#x2F;</a>
ryanthejugglerabout 11 years ago
I&#x27;ve never been exposed to Phabricator before today, but its site[1] reminded me distinctly of <a href="http://xkcd.com/1293" rel="nofollow">http:&#x2F;&#x2F;xkcd.com&#x2F;1293</a> . Pretty sure Phabricator is maintained by Beret Guy.<p>[1] <a href="http://phabricator.org/" rel="nofollow">http:&#x2F;&#x2F;phabricator.org&#x2F;</a>
akurilinabout 11 years ago
What are the various 2014 git code review tools out there one can look into besides Phabricator?
评论 #7697630 未加载
评论 #7697638 未加载
评论 #7697452 未加载
评论 #7697803 未加载
评论 #7698775 未加载
评论 #7698178 未加载
评论 #7697815 未加载
davexunitabout 11 years ago
One of my biggest problems with pull requests is the tendency to have to merge a bunch of &quot;Fix typo&quot;, &quot;Refactor foo&quot; style commits.<p>I prefer the workflow for sending patches on mailing lists: Email patch, get feedback, make new commits to address feedback, rebase (important step), repeat.
评论 #7697991 未加载
评论 #7700086 未加载