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

[Colomban Wendling]

Le 07/07/2015 02:13, Matthew Brush a écrit :
> 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, […]

Yeah, that annoyed the hell out of me more than a few times.

> 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.

Yes.

> 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.

Also true.  For that part they could provide a diff between the previous
and current state, so at least we could see what changed.  But well,
it's not (yet) the case.

> 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.

Well, true but you can also just create "versioned" local branches -- so
you can even diff them.  I generally do that with largish stuff, like:

	<user>/<pr>/v1
	<user>/<pr>/v2
	<user>/<pr>/v3

etc.  Of course it requires manual intervention, but it's not very hard.

> I propose that we disallow force pushing, rebasing, squashing, etc.
> on any PR until it is fully ready for final merging. […] ready for
> merging. At that point it _might_ make sense to fudge history and get
> rid of some noisy "fixup" commits[0].

Agreed, this should be the default behavior.  We should at least try and
see how life gets easier.

> Thoughts, opinions?
> 
> If it sounds like a good idea, I could probably try and update any 
> relevant documentation (HACKING comes to mind) to make a note of
> this[1].
> 
> Cheers, Matthew Brush
> 
> [0]: I personally don't think it's a big deal leaving the history to 
> match real life, but I can see how "Fix some typo" type of commits 
> aren't very useful to keep around.

Well, IMO it doesn't make sense to keep endless fixup commits in the
final marge.  Not only it clobbers history, but it also makes it a lot
harder to bisect.

Of course, this applies to *fixups*, not incremental development.  If
changes were made incrementally in the PR it probably makes sense (or
may not, depending on the case) to keep the incremental history too.

Regards,
Colomban