[Geany-Devel] [RFC]: No force push policy on Github PRs

[Thomas Martitz]

Hello,

first of all, I think github should fix this problem, instead of 
enforcing a suboptimal workflow on us.
I reported this problem to github, let's see if they respond.

Am 07.07.2015 um 02:13 schrieb Matthew Brush:
> Hi All,
>
> As anyone trying to follow Pull Requests on Github has probably 
> noticed, when you force push to your PR branch, Github deletes various 
> comments related to the PR, depending on what got clobbered (it seems).

Not always. I haven't found a consistent pattern, but it seems worse 
when commits are removed.

>
> This makes it extremely difficult to keep track of and finally merge 
> PRs because issues that may have been raised are gone and it leaves 
> holes in the comments which are a useful way to make sure any issues, 
> notes, ideas, etc. have been dealt with before merging.
>
> In addition to the dropped comments, it makes it harder to follow the 
> changes made since it clobbers the Git history too, so you have to 
> basically review the entire changset by looking at the whole diff of 
> all files affected by the PR at once.

Well, assuming an updated PR only changes stuff which has  been 
commented on before or are otherwise explicitely noted in a new comment, 
you do not have to review the entire diff again. And you have to review 
those parts of the diff you commented on again, other parts should be 
fine since they received no comment at the first review, right?

So it boils down to lost comments are the main problem.

>
> Another reason to avoid is because it makes it harder to test a PR if 
> the repos history keeps getting munged, it breaks your previous pulls.
>
> I propose that we disallow force pushing, rebasing, squashing, etc. on 
> any PR until it is fully ready for final merging. We could probably 
> use a label or milestone or something to signify that a PR has been 
> fully reviewed and is ready for merging. At that point it _might_ make 
> sense to fudge history and get rid of some noisy "fixup" commits[0].
>

The more fixup commits the less likely that the post-review cleanup is 
actually going to happen. The largeish linkage-cleanup branch was almost 
merged as is, and I'm sure bisection of in the middle of those commits 
is harder or even impossible.

> Thoughts, opinions?

I prefer rebasing and rewriting commits, because that makes my life 
easier too. I can handle my stuff better if it has a clean history, and 
it helps me in making design decisions because I try to logically 
separate stuff in the commits. And merging master into my PR when the PR 
should eventually be merged into master is not acceptable for, therefore 
I rebase.

Continuously rewriting history is common in the patch distribution via 
mailing lists so it's successfully performed elsewhere. It's really just 
github being bad at maintaining comments and that should be fixed on 
their side.

PS: Lost comments should still be in your mailbox as a last resort, 
since github sends notifications for each.

Best regards.