No matter how much they do, people will still complain.
More code review tools
71–80 of 151 posts
Re: More code review tools
#72Does 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!)
In that case, is there any plans for your processes to change to get a more rapid release-cycle? Going dark for N months to implement a feature seems to fly in the face of the workflow GitHub tends to inspire.
Re: More code review tools
#73Earlier quoted context omitted.
isn't this what the "view outdated diff" button does, or am I misunderstanding?
That button only appears when you add new commits to the existing branch/PR. If you amend any commits and force push, the rewritten commits are lost forever on the GitHub side of things. You can only find them in your local ref-log at that point. Gerrit instead retains each rewrite of the "same" commit. It does this by requiring you to insert a "Change-Id: ..." footer into each commit message (it provides a repo hook…
Only comments on the commits are lost. Comments on the Files tab of the PR itself are kept and you can "View outdated diff".
In our project we exclusively use comments on the Files tab precisely for that reason.
Re: More code review tools
#74Sometimes when I submit a PR and I get a lot of feedback I get lost making sure I've addressed every comment. My #1 feature request would be being able to mark comment threads as resolved. The outdated comment feature sometimes works for this use case but mostly it doesn't.
I didn't see it mentioned but it looks like you can now 'react' to comments, a la facebook. So you could choose one reaction, say, "Horray!", and use it as a marker that you resolved it.
Now I wonder if the commenter is notified on every reaction. The only thing that might make this more difficult.
Re: More code review tools
#75Earlier quoted context omitted.
It's interesting that you mention checking off of review points. I had begun to do that in an informal fashion by updating my pull request with notes in the form of a checklist: - [x] Refactor _ - [ ] Rename variable x - [ ] ... Now that you mention it, it would be very nice to have something like Google Docs's ability to mark comments as resolved.
Unfortunately that doesn't work for us for 2 reasons - 1. There's a data loss bug when multiple people edit the PR descriptions. I've had that as an open issue with GitHub for ~6 months, my team experiences it several times a week. 2. We use the checkboxes to assign and track reviewers, and since there's only one count of checkboxes, it would mess with our "2 of 5 complete" kind of metric for reviews. I'd like first-…
You might be interested in taking a look at PullApprove (https://pullapprove.com/) -- basically you put a YAML in your repo that defines what code review looks like for your team (who, when, how many need to approve, etc. -- http://docs.pullapprove.com/). Approval/rejection can then be triggered by PR comments and it uses the status API so you know when the PR has passed review.
Code review "rules" can get pretty complicated since everyone works a little differently...not sure if GitHub will ever try to tackle that or not but we've got a lot of customers who are happy with how PullApprove fills the gap.
Re: More code review tools
#76I'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.
This is my biggest feature request here as well. I'll often write a lot of small comments inline in the diff, and a bigger comment on the PR itself tying everything together. It'd be nice to be able to submit it all at once, with the ability to reference inline comments in my main comment.
Re: More code review tools
#77It 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…
Hey, I'm one of the devs for this new PR stuff. This flow is something we're hoping to improve as well. Force push support is pretty second class right now, any previous commits and discussion basically gets lost. It sucks. So we already use a internal refspec for tracking the latest PR HEAD. You can actually manually fetch this under `refs/pull/123/head`. However, this is only the latest HEAD, not any previous histo…
Even with these new changes are comments on commits that are force pushed over still lost? It looks like when commenting on individual commits now they appear as if you commented on the whole changeset itself.
Re: More code review tools
#78I'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.
Are 200 line diff common? I feel like having focused patches are useful for everyone. Including git bisect and CI/CD.
Re: More code review tools
#79I 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.
We've been doing exactly that at Omniref: https://www.omniref.com
Re: More code review tools
#80I'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.
Are 200 line diff common? I feel like having focused patches are useful for everyone. Including git bisect and CI/CD.
I read it as a separate diff 200 lines later. But I guess pull requests could easily have that big diffs in a public repo