Hacker News new | ask | show | jobs
by zeotroph 703 days ago
How do you envision pushing/reviewing a branch of 3 cleanly separated commits (say: write failing test, fix code, refactor) into a repo with a fast-forward/no-merge policy? The end result should of course be 3 commits, but review history should not be lost.

1) push 3 commits (author)

2) push 3 commits + 1 review commit (reviewer)

or even 2a), + 3 interleaved review commits, might be needed if the final refactor removes something.

then

3a) force-push 3 commits with the review comments applied (you mentioned --force is supported)

3b) or push 3 + 1 + (worst case 3) fixup commits, then a squash later?

If 3a), does pico have the concept of patchsets like gerrit, so the state 2) can be retrieved later?

PS: ... or even 2c, if the line I am commenting on is removed in patch 2 I would need to push a whole "review tree" (or fix conflicts in b and c):

   a -- b -- c -- review-c
    \   \
     \   `-review-b
      `- review-a
2 comments
qudat 703 days ago
Right now, when force pushing, we preserve event logs for every server mutation, but the review themselves are lost.

We do see this as a problem and are thinking about ways to address it. We do support the idea of a patchset (that's what the patch request managed). Right now we only support a single patchset for a patch request, but we are leaning toward introducing patch revisions where both contrib and owner submit entire versions of the patchset. This could provide better tracability since reviews would be in a separate patchset "channel" and not lost to force pushing.

We also plan on supporting a patch request "cover letter" which includes all the events and comments during review. This cover letter is just an empty patch which could be `git am --empty=keep` or `git am --empty=drop` depending on preference.

link
zeotroph 703 days ago
Good to hear, Gerrit does a lot of things right, see "stacked diffs" (or the realization that GitHub lacks these) now becoming more popular. And the patchset model is one important aspect of that, especially being able to see the difference just between patchsets - which git-pr can then do via `git range-diff` (which you already mentioned).

And even if everything git-pr does could be done by talking to the Gerrit REST-API, a simple straight forward and lightweight implementation of the "essence" of this model is very useful.

link
nektro 703 days ago
they should be done as separate patches
link