7 ms·

When people talk about killer features missing in Git, there is more beyond the UX and mono/poly repo. One thing is code review. There is no code review in Git

When people talk about killer features missing in Git, there is more beyond the UX and mono/poly repo. One thing is code review. There is no code review in Git. What I expect in 2020 is that I should be able to specify reviewers for the commit (which I pick out of a list of people who can approve it). These people should be able to leave comments on the commit. I should be able to both respond to comments and modify the code before the commit gets checked in. The history of the comments and changes should be maintained. There is nothing in Git that supports this flow in a natural way. A replacement for (or evolution of) Git can be a tool that would support this code review flow from the get-go. And yes, there are external tools for code review. But that all should be a part of version control.

tick_tock_tick 7y ago
I think it doesn't exist because there is no demand for that to be part of the codes history. Nothing you've described sounds very useful after a month or so.
- romwell 7y ago
  Sure, but keeping that history is just a small part of what I said. The point about code review still stands.
  - rhabarba 7y ago
    That's the one major point where Fossil probably falls short: Extending a repository's capabilities would add overhead to every single one of them (and, potentially, to each commit's meta data).
- pbh101 7y ago
  All this talk of Git-sympathetic code review tools and nobody has mentioned Gerrit, which seems at least somewhat close to what is being described. Each patchset of each review is its own Git ref and is often referenced in the final commit. Separately, I know many including myself who would love for code review comments to more seamlessly be integrated into the code browsing experience.
  - hinkley 7y ago
    People also seem to be conflating review comments associated with lines of code with an opinionated code review tool. You can keep the banter with the code and the go-no-go decision separate, even external. But post mootems have worked better when someone realized that one of the team repeatedly calls out a class of errors that bite us later and they’re being ignored. You have the ability to prevent this error. Wise up or that person will decide rightfully that we are a bunch of clowns and leave.
dhash 7y ago
Iron [1] definitely has the right idea. Code review comments really should live next to the code, and proper editor integration goes a really long way. Traditional review processes make “just fixing the code” have a very high inertia. [1] https://blog.janestreet.com/ironing-out-your-release-process/ https://blog.janestreet.com/ironing-out-your-release-process...
- aledalgrande 7y ago
  Thanks for this one, really nice read.
aylons 7y ago
I don't think this should be part of version control, simply because is too tied to the environment and development practices that may not be shared by the whole set of current and future developers of any given project. The version control should keep the code history, not the paperwork history. What I think you're looking for could, however, use git as a platform for that. That's what GitHub, GitLab and the likes do using a web interface and there's enough extensibility and power on git for command-line or desktop tools to do it. The internals of git are very much akin to a filesystem, by the way, and the plumbing gives you more enough access to use that in creative ways. From the top of my head, maybe a system like this could automatically generate tags for code reviews, use merges and branching for answering to these reviews, and hooks for notifying interested parts on particular areas of code. All while the messages and reviews themselves travel on another data layer, which references git but does not mingle with it. This separation (and even tag cleaning, for example), would be specially useful on huge distributed projects where a company or small team may have whatever development process it needs internally and sharing only the results without having to completely hide the code history.
- deanCommie 7y ago
  I think the distinction you're drawing between code history and paperwork history is more arbitrary than you give it credit. If all we cared about was code history, a super pure "version control" system would have one trunk, no branches, and a sequentially increasing version number with no commit messages or author information. But if you can annotate an entire commit with a descriptive message, why not annotate a specific hunk of code changes (comments), or the discussion over those changes in a specific version? Environments and development practices may change, but the code review that led to a particular code decision at some point in history are a valid representation of the context at that time in history. Don't get me wrong, I'm not lacking these features in Git, and am happy to get them from other platforms like Github, but I think the comment you're responding to is astute that one can imagine a git replacement that incorporates code review functionality as a first-tier feature.
  - yjftsjthsd-h 7y ago
jayd16 7y ago
I disagree. Git handles this through the power to push/pull and muddying that with a bunch of alternative baked in user flows seems like a mistake. Third party software can handle adding meta-information to pull requests.
earhart 7y ago
Something like https://github.com/google/git-appraise https://github.com/google/git-appraise ?
donw 7y ago
> And yes, there are external tools for code review. But that all should be a part of version control. There are many different code-review workflows, and for teams that do full-time pair-programming, code review happens in real-time as the code is written. Trying to bake support for all of that into something that is also a good VCS sounds like a recipe for one of those clock-birdfeeder-machete-flashlight-massager tools that you used to see on the back pages of outdoor magazines in the 90s.
- grive 7y ago
  e.g. -- fossil. This is exactly the issue with it: its author(s) lament that git has not all of these things integrated, but they are ignoring that this is exactly why fossil cannot gain a wider adoption. It has opinions about how the team and the project should be run / managed / documented. This is not for the tool dev to have these opinions
mcv 7y ago
> "There is no code review in Git." Yes there is. You can block pushing directly to a branch of the repository and demand that things can only be merged through pull requests. You code review the pull requests. This is very well supported by Atlassian's git tooling, for example. It does everything you mention. It's true that the comments don't become part of the git repository itself, but I'm not convinced they should be.
blt 7y ago
Sounds like the Google workflow.

tick_tock_tick 7y ago

I think it doesn't exist because there is no demand for that to be part of the codes history. Nothing you've described sounds very useful after a month or so.

romwell 7y ago
Sure, but keeping that history is just a small part of what I said. The point about code review still stands.
- rhabarba 7y ago
  That's the one major point where Fossil probably falls short: Extending a repository's capabilities would add overhead to every single one of them (and, potentially, to each commit's meta data).
pbh101 7y ago
All this talk of Git-sympathetic code review tools and nobody has mentioned Gerrit, which seems at least somewhat close to what is being described. Each patchset of each review is its own Git ref and is often referenced in the final commit. Separately, I know many including myself who would love for code review comments to more seamlessly be integrated into the code browsing experience.
- hinkley 7y ago
  People also seem to be conflating review comments associated with lines of code with an opinionated code review tool. You can keep the banter with the code and the go-no-go decision separate, even external. But post mootems have worked better when someone realized that one of the team repeatedly calls out a class of errors that bite us later and they’re being ignored. You have the ability to prevent this error. Wise up or that person will decide rightfully that we are a bunch of clowns and leave.

dhash 7y ago

Iron [1] definitely has the right idea. Code review comments really should live next to the code, and proper editor integration goes a really long way. Traditional review processes make “just fixing the code” have a very high inertia. [1] https://blog.janestreet.com/ironing-out-your-release-process/ https://blog.janestreet.com/ironing-out-your-release-process...

aledalgrande 7y ago
Thanks for this one, really nice read.

aylons 7y ago

I don't think this should be part of version control, simply because is too tied to the environment and development practices that may not be shared by the whole set of current and future developers of any given project. The version control should keep the code history, not the paperwork history. What I think you're looking for could, however, use git as a platform for that. That's what GitHub, GitLab and the likes do using a web interface and there's enough extensibility and power on git for command-line or desktop tools to do it. The internals of git are very much akin to a filesystem, by the way, and the plumbing gives you more enough access to use that in creative ways. From the top of my head, maybe a system like this could automatically generate tags for code reviews, use merges and branching for answering to these reviews, and hooks for notifying interested parts on particular areas of code. All while the messages and reviews themselves travel on another data layer, which references git but does not mingle with it. This separation (and even tag cleaning, for example), would be specially useful on huge distributed projects where a company or small team may have whatever development process it needs internally and sharing only the results without having to completely hide the code history.

deanCommie 7y ago
I think the distinction you're drawing between code history and paperwork history is more arbitrary than you give it credit. If all we cared about was code history, a super pure "version control" system would have one trunk, no branches, and a sequentially increasing version number with no commit messages or author information. But if you can annotate an entire commit with a descriptive message, why not annotate a specific hunk of code changes (comments), or the discussion over those changes in a specific version? Environments and development practices may change, but the code review that led to a particular code decision at some point in history are a valid representation of the context at that time in history. Don't get me wrong, I'm not lacking these features in Git, and am happy to get them from other platforms like Github, but I think the comment you're responding to is astute that one can imagine a git replacement that incorporates code review functionality as a first-tier feature.
- yjftsjthsd-h 7y ago

jayd16 7y ago

I disagree. Git handles this through the power to push/pull and muddying that with a bunch of alternative baked in user flows seems like a mistake. Third party software can handle adding meta-information to pull requests.

earhart 7y ago

Something like https://github.com/google/git-appraise https://github.com/google/git-appraise ?

donw 7y ago

> And yes, there are external tools for code review. But that all should be a part of version control. There are many different code-review workflows, and for teams that do full-time pair-programming, code review happens in real-time as the code is written. Trying to bake support for all of that into something that is also a good VCS sounds like a recipe for one of those clock-birdfeeder-machete-flashlight-massager tools that you used to see on the back pages of outdoor magazines in the 90s.

grive 7y ago
e.g. -- fossil. This is exactly the issue with it: its author(s) lament that git has not all of these things integrated, but they are ignoring that this is exactly why fossil cannot gain a wider adoption. It has opinions about how the team and the project should be run / managed / documented. This is not for the tool dev to have these opinions

mcv 7y ago

> "There is no code review in Git." Yes there is. You can block pushing directly to a branch of the repository and demand that things can only be merged through pull requests. You code review the pull requests. This is very well supported by Atlassian's git tooling, for example. It does everything you mention. It's true that the comments don't become part of the git repository itself, but I'm not convinced they should be.

blt 7y ago

Sounds like the Google workflow.