+1 to pushing PR changes as separate commits. Also +1 to creating PRs with multiple commits where it makes sense.
-Dan On Fri, May 31, 2019 at 1:28 PM Owen Nichols <onich...@pivotal.io> wrote: > Personally, I do not force-push to my PRs once any review comments have > been accumulated, for the reasons you mention. > > Not sure if some people just force-push out of habit, or if the > requirement for initial commit to be squashed creates some fear. > > I would go a step further and suggest that there is no reason to even > require the initial PR to be a single, squashed commit. If you have made a > small fix and also done some refactoring or cleanup, as a reviewer I would > much rather you submit that PR with multiple distinct commits, so I don’t > have to comb through a gigantic diff trying to spot what is the real change > and what is just renames. > > GitHub already very nicely presents the combined delta by default, as if > it was a single squashed commit, so to me there is only negative value in > encouraging/requiring pre-squashing. > > > On May 31, 2019, at 1:20 PM, Darrel Schneider <dschnei...@pivotal.io> > wrote: > > > > Something I have noticed is that often when I have requested changes be > > made to a pull request is that the changes are force pushed ask a single > > commit to the pr. I would actually prefer that the changes show up as a > new > > commit on the pr instead of everything being rebased into one commit. > That > > makes the history of the pr easier to follow and make it easy to see what > > has changed since the previous review. What do others think? Have we done > > something that makes contributors think the pull request has to be single > > commit? I know the initial pull request is supposed to be but from then > on > > I'd prefer that we wait to squash when we merge it to develop. > >
