I really don't like force-push to PRs after comments have been made. At least GitHub shows that the force-push occured now, but that still blocks us from seeing the incremental change.
For needing to update a PR due to develop having progressed, I recommend using `git merge origin/develop <my-branch>` rather than a rebase. For merging back to develop, I am torn between squash (one commit per feature) and rebase-merge (linearizes history a-la SVN). I like the idea of true-history playing out in our code, but I understand Naba's point that squashed commit-per-feature is easier to bisect. My 2c On Fri, May 31, 2019 at 1:44 PM Jacob Barrett <jbarr...@pivotal.io> wrote: > Would that be PR of a the PR template. Might that cause a black hole to > form? > > I’m in favor of updating this wall of text you smack your face on for each > PR. Let’s pair it down to discourage people (myself strongly included) from > ignoring or deleting it. > > -jake > > > > On 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. > >>> > > >
