Comment by ivanjermakov

Comment by ivanjermakov 4 days ago

I find the idea of using git for code reviews directly quite compelling. Working with the change locally as you were the one who made it is very convenient, considering the clunky read-only web UI.

I didn't get why stick with the requirement that review is a single commit? To keep git-review implementation simple?

I wonder if approach where every reviewer commits their comments/fixes to the PR branch directly would work as well as I think it would. One might not even need any additional tools to make it convenient to work with. This idea seems like a hybrid of traditional github flow and a way Linux development is organized via mailing lists and patches.

spike021 4 days ago

is github's PR considered read-only?

i've had team members edit a correction as a "suggestion" comment and i can approve it to be added as a commit on my branch.

Reply View 3 replies

ivanjermakov 4 days ago

By read-only I meant that you can't fully interact with the code: run/debug it, use intellisense, etc.

Reply View | 2 replies
- spike021 4 days ago
  
  Can't you just check out the branch of the repo locally into your ide? i'm still confused what limitation you are talking about.
  
  Reply View | 1 reply
  
  ivanjermakov 3 days ago
  
  Described well in the post. This way you have to switch between ide and web diff viewer, redundant and not convenient.
  
  Reply View | 0 replies

teiferer 4 days ago

> I didn't get why stick with the requirement that review is a single commit

Yeah that is pretty weird. If 5 people review my code, do they all mangle the same review commit? We don't do that with code either, feels like it's defeating the point.

Review would need to be commits on top of the reviewed commit. If there are 5 reviews of the same commit, then they all branch out from that commit. And to address them, there is another commit which also lives besides them. Each commit change process becomes a branch with stacked commits beinf branches chained on top of one another. Each of the commits in those chained branches then has comment commits attached. Those comment commits could even form chains if a discussion is happening. Then when everybody is happy, each branch gets squashed into a single commit and those then get rebased on the main branch.

You likely want to make new commits for that though to preserve the discussions for a while. And that's the crux: That data lives outside the main branch, but needs to live somewhere.

Reply View 0 replies