Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

More code review tools

github.com

Post reply on HN
Print view View original

31–40 of 151 posts

Re: More code review tools

#31

Post by kragniz »

nightpool wrote: post #28
js2 wrote: post #21

It 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…

isn't this what the "view outdated diff" button does, or am I misunderstanding?

Can you diff between multiple outdated diffs? One of the really useful things in the gerrit workflow is to check what's changed since you last reviewed a patchset.

Re: More code review tools

#32

Post by nightpool »

alexwebb2 wrote: post #10

Did 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.

Re: More code review tools

#33

Post by marcinkuzminski »

js2 wrote: post #21

It 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…

This is what we're actually making inside RhodeCode, we already have explicit PR updated, and we actually version pull requests, and each update of it. Imho pull requests is a set of changes and you need to track how it evolved.

Re: More code review tools

#34

Post by nightpool »

willchen wrote: post #6

Does 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!

The problem with github is that every time something about them comes up, everybody has their own ideas about what the "most critical" thing that Github "needs" to do is. Tracking and collecting those, doing their own user studies to find pain points, making sure you're not hurting existing behavior, all of this stuff takes a lot of time to get right. I'm not surprised that they have long fallow periods followed by quick bursts of updates.

Re: More code review tools

#35

Post by marcinkuzminski »

danpalmer wrote: post #26
faitswulff wrote: post #9

Earlier 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-…

This is exactly how voting, and reviewers concept works in RhodeCode. You have accept/reject/under review states for each reviewer, and you have to make full voting in order to merge a PR.

Re: More code review tools

#36

Post by nightpool »

danpalmer wrote: post #26
faitswulff wrote: post #9

Earlier 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-…

Question: why do you use checklists to assign reviewers, when github has that functionality built in?

Re: More code review tools

#37

Post by Negative1 »

Nice additions. Can't help but feel at some point you'll be able to edit and compile code directly on Github (with some sort of compiler/CL backend). Has Github ever discussed integrating Atom directly into the site?

Re: More code review tools

#38

Post by xPaw »

erichurkman wrote: post #7
danpalmer wrote: post #2

This is a great step in the right direction. I use GitHub for code review every day, and it has historically been very poorly designed for thorough reviews. These changes look great, I just hope that we get some sort of checking-off of review points, and accept/reject functionality.

I'd love a UI way to enable `?w=1` whitespace mode, especially for html/json/yaml files. I have a feeling a lot of reviewers don't know about it and spend an inordinate amount of time squinting to see what changes when it was a whitespace-only change.

I have a userscript that does just that, however it needs to be updated to account for these new review tools.

https://gist.github.com/xPaw/de6ee132a2e267ef6960

Re: More code review tools

#39

Post by owensims1 »

hwangmoretime wrote: post #13

Code review is something all of us do, but all of us do differently. Anyone know of any nice frameworks, articles, or blog posts for code review? I'm particularly interested in the case where knowledge transfer is a high priority in the code review. My current side project integrates feedback theory [1], to provide scaffolds and other cues to help and remind reviewers to give high quality feedback. Thus, my interest.…

I wrote this high-level overview of how my team does code review, with an emphasis on knowledge transfer: http://engineering.zesty.com/code_review/. Hope you enjoy.

Re: More code review tools

#40

Post by artursapek »

Awesome to see GitHub stepping to the plate with these updates. IMO the biggest thing missing in the PR's diff view is a git blame column beside the changes, so if someone is writing good commits I can glance through the entire diff and see which changes were introduced together and why.

Just a feature request, and I know how annoying those can be on the receiving end. Great updates either way.

Post reply on HN