+1 for initial PR needs to be a single commit +1 for making intermittent commits to the PR to show history of revision.
I would only do a rebase and force-push if github tells me that there is a conflict, but my rebase would keep my original commits (no squashing). I do not like to do a merge, since it will pull in other people's commits to my PR. On Fri, May 31, 2019 at 1:33 PM Anthony Baker <aba...@pivotal.io> wrote: > Let’s update the checklist to match the outcome of this thread: > > https://github.com/apache/geode/blob/develop/.github/PULL_REQUEST_TEMPLATE.md > < > https://github.com/apache/geode/blob/develop/.github/PULL_REQUEST_TEMPLATE.md > > > > Anthony > > > > On May 31, 2019, at 1:31 PM, Helena Bales <hba...@pivotal.io> wrote: > > > > +1. I would guess that it is the checklist as part of the PR that is > > confusing people. > > > > The other reason that history gets rewritten is when force pushing after > a > > rebase. While fast-forwarding is necessary on occasion, this can be > > accomplished without rewriting history by using a merge. > > > > As part of our document on making PRs, we should include instructions on > > how to handle the situation where fast-forwarding is necessary, > explicitly > > discourage the use of merges and force-pushes once a PR has been opened, > > and some guidelines regarding the appropriate number of commits when the > PR > > is initially opened. Once we have these guidelines, it would be helpful > to > > link to them from the PR checklist that we currently have, and rework the > > checklist so that it is in line with our desired process. > > > > On Fri, 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. > >> > > -- Cheers Jinmei
