Hacker News new | ask | show | jobs
by XorNot 998 days ago
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.
2 comments
da39a3ee 998 days ago
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.
link
XorNot 997 days ago
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.

link
da39a3ee 997 days ago
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.
link
XorNot 997 days ago
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.

link
da39a3ee 997 days ago
Right, agree with all that.
link
zerbinxx 998 days ago
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
link