I wish it were possible to put comments directly on a line of code in any commit / repo outside of a PR. Sometimes someone wants me to review their code that they aren't submitting anywhere.
More code review tools
41–50 of 151 posts
Re: More code review tools
#42I'd love it if there was a way for me to queue my comments before submitting. I often keep my comments in a separate textedit/nv window and then go back to put them in since I want to keep track of questions that arise as I'm reading, but I don't want to pepper someone with comments that would be resolved 200 lines later in the diff.
Can't you just enter your comment into the text box but not hit the "Comment" button until you've gotten through the whole PR? I just gave it a shot, and it looks like you can have multiple comment text boxes open simultaneously.
Re: More code review tools
#43If next to every file, Github also listed its filesize, I would know exactly what file to begin looking at when I encountered a new repo (to a good approximation -- in any case, I would be able to intuit better decisions with a combination of filenames, the information in hypothetical README.md's, and filesizes than just READMEs and filenames alone..)
Maybe this doesn't scale or something? It's something I've felt necessary for a while though. Maybe their user testing has concluded otherwise?
Re: More code review tools
#44It would be nice if GitHub supported a Gerrit-inspired code-review process, where instead of having to choose between: 1) piling new commits onto the existing branch/PR, or 2) force-pushing and completely losing the old commits on the server You could instead push into a "magic" ref-spec and the server would retain the original commits, but also the rewritten commits, such that a PR can have multiple revisions. This…
Phabricator does this too, but it works even if you force-push to the same branch - it lets you compare the base commit against all subsequent (force-)pushed commits onto the same ref.
Re: More code review tools
#45It would be nice if GitHub supported a Gerrit-inspired code-review process, where instead of having to choose between: 1) piling new commits onto the existing branch/PR, or 2) force-pushing and completely losing the old commits on the server You could instead push into a "magic" ref-spec and the server would retain the original commits, but also the rewritten commits, such that a PR can have multiple revisions. This…
Phabricator does this too, but it works even if you force-push to the same branch - it lets you compare the base commit against all subsequent (force-)pushed commits onto the same ref.
Cool. Gerrit requires that each commit message have a "Change-Id: ...." footer so that it can pair up the new incoming commits with the old ones. And you have to push into its magic ref-spec namespace. Neat that Phabricator works by force-pushing over the existing branch, and just keeps track of its previous SHA1s, sorta like a server-side ref-log, exposed via its UI.
Re: More code review tools
#46I'd love it if there was a way for me to queue my comments before submitting. I often keep my comments in a separate textedit/nv window and then go back to put them in since I want to keep track of questions that arise as I'm reading, but I don't want to pepper someone with comments that would be resolved 200 lines later in the diff.
Can't you just enter your comment into the text box but not hit the "Comment" button until you've gotten through the whole PR? I just gave it a shot, and it looks like you can have multiple comment text boxes open simultaneously.
And yet it will still send emails at the end, and you lose progress if you close tab
Re: More code review tools
#47Did they kill commit-level comments? I can no longer make comments on a given commit - the entry box at the bottom is gone. Looks like you have to scroll all the way back up to the top, switch to the "Conversation" tab, and then make a PR-level comment instead of a commit-level comment. I hope I'm missing something here, because as it stands now it's a big step backwards.
They removed full-commit comments on PRs, as opposed to line-level comments on either commits or diffs, or full-commit comments on non-PR commits, but I don't think anyone used those—I haven't even seen anyone use something that wasn't a line comment for a long, long time.
Line-level comments trigger an absurd number of notifications, and there's no way to batch them. We abandoned that idea within a day.
PR-level comments lack any sort of context - there's no code at all unless you manually link it.
Commit-level comments were a sane, reasonable compromise, and now they're gone.
Your own workflow is obviously going to be different than mine, and it's good that you aren't affected by this, but hopefully you can relate to my disappointment with the way they've handled this. How would you feel if a product you were paying for suddenly dropped support for a feature you rely on without any notice or explanation?
Re: More code review tools
#48It would be nice if GitHub supported a Gerrit-inspired code-review process, where instead of having to choose between: 1) piling new commits onto the existing branch/PR, or 2) force-pushing and completely losing the old commits on the server You could instead push into a "magic" ref-spec and the server would retain the original commits, but also the rewritten commits, such that a PR can have multiple revisions. This…
Have you tried addressing the comments with "fixup!" commits? At the end of review, you can close the pull request, "rebase --autosquash --interactive" the branch, and merge manually.
I use fixup commits locally, and rewrite before pushing. I never considered pushing the fixup commits to the server. That's just another work-around, but an interesting one.
Re: More code review tools
#49I'd love it if there was a way for me to queue my comments before submitting. I often keep my comments in a separate textedit/nv window and then go back to put them in since I want to keep track of questions that arise as I'm reading, but I don't want to pepper someone with comments that would be resolved 200 lines later in the diff.
As you enter your comments just don't click the "comment" button. Github saves the text anyway as you go. I don't know how long it takes to save the text (so maybe don't refresh or leave the instant you finish writing a comment) but in general they should all be there. Then you just have to scroll through and click comment on each one.
Certainly not as easy as seeing them in a nice row for review but it's a workaround at least.
Re: More code review tools
#50Does anybody else feel like GitHub has released more features in the last month than the last 6 months? I'm not sure if it's just a coincidence with all the attention they've gotten on HN, but these improvements are much appreciated!
Code review tooling has been in the works for a long time. It's sadly taking us this long to actually ship the first fundamental bits, but there's a ton that this unlocks for us. Expect to see more soon :).
(Source: I worked on code review at GitHub for awhile!)