geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Owen Nichols <onich...@pivotal.io>
Subject Re: what is the best way to update a geode pull request
Date Fri, 31 May 2019 20:36:27 GMT
I’ve almost never seen a PR where this checklist was filled out.  Either the PR is created
with the original boilerplate intact (or sometimes it’s deleted entirely).

I feel like this wall of boilerplate discourages contributors from instead using that space
to describe their change and add other context that would be helpful to a reviewer...


> On May 31, 2019, at 1:33 PM, Anthony Baker <abaker@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 <hbales@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 <dschneider@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.
>>> 
> 


Mime
View raw message