Hello, thanks for the post, I really enjoyed it and I think there are good points there. In general, I’m a fan of reducing ceremonies and boilerplate as much as possible (without losing control), so this approach fits well that way of thinking.
There’s something I didn’t understand, do you mind help me fill the gap?
The post says:
“You can't push your commit to master until your changes are approved.
If there are any suggestions, the developer would need to modify the commit (using amend or interactive rebasing) and push it again. This gives a clear view of what was changed and the reviewer understands if their suggestions were incorporated. When the reviewer approves the commit, it can be pushed to master.”
If we only use master, but we can’t push to master, where do we push to at the beginning? Where does the reviewer do its job? I didn’t understand either that we can’t push to master but then we need to amend the commit and push it
I got lost at some point but I don’t know exactly where.
But the basic idea is that you don't actually push to master, you push to a review queue instead, where you can still amend your commit, and when it is finally approved, it is then merged with master.
And yes, this could easily mean that you've got merge conflicts again, so you're back to square one. Which is the Achilles heel of the whole idea, you now have to do the very thing you were supposed to avoid.
There is a difference though.
All this extra work is only needed when a commit fails the review. So if 99% of your commits pass review, all you're doing is pushing to one remote and pulling from another, which takes no extra effort in practice.
But if you're working with a "novice" team who are likely to fail reviews often, this workflow is a nightmare. And if you've only got two or three devs actively working on a codebase, it's probably an overkill.
So the important thing as always, is to think about what works for your specific circumstances, try your idea, then continuously evaluate and refine it.
The implementation may vary but the idea is there is temporary space where your commit is stored, and it's cherry-picked on the target branch when it's approved. This is very similar to how gerrit works. Commits can only be merged in the target branch once it has been reviewed.
The tool I am talking about can probably send a diff to a third party service or can create a temporary branch for your commit. . That'll be an abstraction.
We use Gerrit where I work, and I generally like it. The UI is tolerable with the newer beta design :)
The issue I have with it is that it only allows single commits to be reviewed. I find this to be limiting, as it requires one to amend a commit to address code review feedback. You also lose all the history of the changes in git. Gerrit does store the history of a review as patchsets whenever you push the amended commit with the same Change-Id and provides diffs which is nice, but this history is not stored in git itself (afaik).
For further actions, you may consider blocking this person and/or reporting abuse
We're a place where coders share, stay up-to-date and grow their careers.
Hello, thanks for the post, I really enjoyed it and I think there are good points there. In general, I’m a fan of reducing ceremonies and boilerplate as much as possible (without losing control), so this approach fits well that way of thinking.
There’s something I didn’t understand, do you mind help me fill the gap?
The post says:
“You can't push your commit to master until your changes are approved.
If there are any suggestions, the developer would need to modify the commit (using amend or interactive rebasing) and push it again. This gives a clear view of what was changed and the reviewer understands if their suggestions were incorporated. When the reviewer approves the commit, it can be pushed to master.”
If we only use master, but we can’t push to master, where do we push to at the beginning? Where does the reviewer do its job? I didn’t understand either that we can’t push to master but then we need to amend the commit and push it
I got lost at some point but I don’t know exactly where.
Thanks again for the post!
This is what tools like Gerrit are for.
But the basic idea is that you don't actually push to master, you push to a review queue instead, where you can still amend your commit, and when it is finally approved, it is then merged with master.
And yes, this could easily mean that you've got merge conflicts again, so you're back to square one. Which is the Achilles heel of the whole idea, you now have to do the very thing you were supposed to avoid.
There is a difference though.
All this extra work is only needed when a commit fails the review. So if 99% of your commits pass review, all you're doing is pushing to one remote and pulling from another, which takes no extra effort in practice.
But if you're working with a "novice" team who are likely to fail reviews often, this workflow is a nightmare. And if you've only got two or three devs actively working on a codebase, it's probably an overkill.
So the important thing as always, is to think about what works for your specific circumstances, try your idea, then continuously evaluate and refine it.
The implementation may vary but the idea is there is temporary space where your commit is stored, and it's cherry-picked on the target branch when it's approved. This is very similar to how gerrit works. Commits can only be merged in the target branch once it has been reviewed.
The tool I am talking about can probably send a diff to a third party service or can create a temporary branch for your commit. . That'll be an abstraction.
We use Gerrit where I work, and I generally like it. The UI is tolerable with the newer beta design :)
The issue I have with it is that it only allows single commits to be reviewed. I find this to be limiting, as it requires one to amend a commit to address code review feedback. You also lose all the history of the changes in git. Gerrit does store the history of a review as patchsets whenever you push the amended commit with the same Change-Id and provides diffs which is nice, but this history is not stored in git itself (afaik).