61–70 of 224 posts
When doing code reviews, I think it is annoying that every time I comment on a line, PR author gets a notification. This is not a simultaneous, real-time thing. I'm in the middle of doing my review, and my comments are not ready to be read. Maybe I'll change my mind on my comment on line 8 when I reach line 80.
Ha, that's funny. But yes, please we need interdiffs in GitHub and GitLab. I want to have my PRs/MRs always rebased and I don't want "fix code review comments" commits.
When doing code reviews, I think it is annoying that every time I comment on a line, PR author gets a notification. This is not a simultaneous, real-time thing. I'm in the middle of doing my review, and my comments are not ready to be read. Maybe I'll change my mind on my comment on line 8 when I reach line 80.
GitLab lets you compose the entire review before submitting it so the author won't see anything until you're done.
Most of the complaints here could be solved by having smaller pull requests and then squashing commits when it’s time to merge.
When doing code reviews, I think it is annoying that every time I comment on a line, PR author gets a notification. This is not a simultaneous, real-time thing. I'm in the middle of doing my review, and my comments are not ready to be read. Maybe I'll change my mind on my comment on line 8 when I reach line 80.
GitLab lets you compose the entire review before submitting it so the author won't see anything until you're done.
It is extra work and not everyone appreciates the benefits, so it’s hard to convince coworkers to do the same.
I recently got introduced to Sapling, specifically to the approach of never merging more than a single commit and also doing code reviews commit by commit. I like that idea even better!
Of course with existing tools like git & GitHub this seems rather difficult to implement, but Sapling automatically keeps track of how commits change across fixups & rebases, and it also handles entire "stacks" of merge requests / commits.
It's always exciting to see new approaches to code reviews - GitHub has its strengths, but it’s far from perfect. For the scenario you’ve outlined, have you thought about splitting the 3 patches into separate, dependent pull requests? While GitHub doesn’t natively support this, the right code review tool (shameless plug - I’m part of a team building one called GitContext) should allow you to keep pull requests small…
As for GitContext, how do you keep track of commits across fixups, rebases, reordering, etc.?
Earlier quoted context omitted.
We use stacked commits + rebase only in our company. The commit history is linear and it's very easy to revert changes. I don't see any advantage of using merging instead of rebase I am not sure why we need to squash commits. We encourage the opposite where you should commit small and often. So if we need to revert any commit, it's less painful to do so.
Without squashing it's hard for me to commit as small and often as I would like. Some things I want out of the final series of commits: 1) everything builds. If I need to revert something or roll back a commit, the resulting point of the codebase is valid and functional and has all passing tests. 2) features are logically grouped and consistent - kinda similar to the first, but it's not just that I want the build to…
For example, if the original commit series was
Do a small refactor before I can start adding the test
Add the test for the feature
Do a small refactor before I can start adding the feature
Work in progress
Complete sub-feature 1
Work in progress
lint
lint
Complete sub-feature 2
Respond to reviewer 1 comments
Respond to reviewer 2 comments
Implement feature
Do a small refactor before I can start adding the test
Do a small refactor before I can start adding the feature
Complete sub-feature 1
Complete sub-feature 2