On Wed, Jul 8, 2015 at 5:47 AM, Matthew Brush <mbr...@codebrainz.ca> wrote: > > > 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]. >> >> The question is how to detect the "fully ready for final merging" moment. For my patches the workflow typically looks like
1. I submit a patch 2. Colomban reviews it 3. I repush the patch with fixes 4. Colomban merges it I kind of implicitly assume that after (3) the patch will be ready for merging (and it typically is) so I do the force push but of course there may be further comments. For bigger patch sets one should choose what seems to be most practical. For instance for https://github.com/geany/geany/pull/488 where there were many commits and also review comments I decided to create a separate pull request containing the fixes https://github.com/geany/geany/pull/505 to preserve the comments in the original pull request. In this case adding fix commits would make things too messy. So personally I wouldn't carve any rules in stone and would decide case to case. For bigger patches with many review comments it's probably best to ask the reviewer which way he prefers to have the fixes committed. Cheers, Jiri
_______________________________________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel