[zerbinxx]

One practice I’ve gotten into with juniors is checking out their PR, branching it, PR-ing to their PR, and then having the discussion about what I want them to change there. This is good because it lets us isolate certain issues (and truly resolve them) rather than comments getting pulverized by lots of little commits, only to have someone else come in and say “LGTM!” and merge it with unresolved issues.

Another obvious way to fix OP’s issue is to request developers to break their PR’s down into smaller chunks, pair on the parts that need help, and slowly merge things from there. YMMV of course - but any 1000 line PR is going to be a headache regardless of what methodology you’re using to review code.

[XorNot]

I like this approach too, but almost no one understands it (i.e. the idea it's possible is foreign) and UI support for it is non-existent in all the major products.

It would be wonderful if we could ditch "change these lines" type comments in favour of just letting merge requests with the changes be easily surfaced.

[da39a3ee]

I might be wrong but it sounds like you don't know about the GitHub PR feature where you select the lines, and GitHub allows you to edit them, and posts a diff when you're done, and if the author accepts it out becomes a jointly authored commit.

[XorNot]

In situations where I'm doing PR reviews seriously in a multi-developer environment I'm usually on Gitlab. Gitlab definitely has some features for this sort of thing, but I've never been able to use them seamlessly or quickly.

I was at one point considering writing a tool which would checkout an MR, then let you just edit it as per normal, and then would submit the whole thing back to a Gitlab MR as a set of proposed changes. The point was to ensure that you could easily expand the MR beyond the diff of changed lines, which was frequently inadequate to review a patch properly since it omitted context.

[da39a3ee]

One way to do that is to open a second PR which targets the branch of the first PR. I think that's also probably the most natural way -- would you agree? I don't see it done much, but I've done it once or twice and I'd embrace it if it were more culturally standard.

[XorNot]

That's what the parent comment is talking about though: and I agree it's better, but it just pops as another global PR.

The UI doesn't know what to do with it, and most developers don't really understand it without spending a bunch of time explaining it.

When it works, it works great but the overhead is ridiculous since you get nothing inline on the UI: ideally something like the commits should pop up as comments, with accept/reject/discuss options.

[da39a3ee]

Right, agree with all that.

[zerbinxx]

Yeah, it really wouldn’t be that hard to implement though. I think I’ll take a stab at something like this using GitHub cli tomorrow or Monday