nifi-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andy LoPresto <alopresto.apa...@gmail.com>
Subject Re: patch and pull request reviewing question
Date Fri, 18 Mar 2016 02:59:48 GMT
Matt Burgess and I were having this conversation last night as well. I prefer to see the incremental
steps of atomic units of work (i.e. commits) in a thread, but understand some people prefer
to review a single squashed commit for a PR. In a feature branch, for example, I would prefer
a way to clone the branch for the PR, keep one branch/snapshot with incremental history but
then only merge a single coherent commit into the master. Unfortunately, the rebase obviously
overwrites that history. Does anyone have suggestions/experience for handling this?

Andy LoPresto
alopresto.apache@gmail.com
PGP Fingerprint: 70EC B3E5 98A6 5A3F D3C4  BACE 3C6E F65B 2F7D EF69

> On Mar 17, 2016, at 7:47 PM, Oleg Zhurakousky <ozhurakousky@hortonworks.com> wrote:
> 
> Going back to T's original point, I'd go as far as asking the following:
> Once the PR is issued, do NOT squash until asked to do so.
> The reason for it is that I just had a great experience with Percival on new JMS support.
There were 6+ follow ups. Joe only needed to review what we agreed/argued, nothing else. Once
he was satisfied he asked me to squash and merged. Done deal.
> The Spring PR (assuming this's thread origin) was a bit different since I somehow thought
that fresh look at the whole thing would be better. My bad!
> 
> That is why I love Git, but went right against its wisdom;)
> 
> Cheers
> Oleg
> 
> Sent from my iPhone
> 
>> On Mar 17, 2016, at 22:21, Aldrin Piri <aldrinpiri@gmail.com> wrote:
>> 
>> Sounds good to here as well.
>> 
>> Given the increasing number of contributions, which predominantly seem to
>> arrive via GitHub PR, I also created NIFI-1615 [1] a little while ago to
>> make a GitHub PR template [1].  Would like to distill the contributor guide
>> down into a few easy steps for folks to check out and make apprised of
>> before they commit the PR that I think can help our reviewers and
>> infrastructure.
>> 
>> [1] https://issues.apache.org/jira/browse/NIFI-1615
>> [2] https://github.com/blog/2111-issue-and-pull-request-templates
>> 
>> On Thu, Mar 17, 2016 at 9:55 PM, Oleg Zhurakousky <
>> ozhurakousky@hortonworks.com> wrote:
>> 
>>> +1 here as well
>>> My apologies Tony
>>> 
>>> Sent from my iPhone
>>> 
>>>> On Mar 17, 2016, at 21:42, Joe Witt <joe.witt@gmail.com> wrote:
>>>> 
>>>> +1
>>>> 
>>>>> On Thu, Mar 17, 2016 at 9:37 PM, Tony Kurc <trkurc@gmail.com> wrote:
>>>>> As I was reviewing a pull request, i realized that it actually was a
bit
>>>>> more mental effort in this instance to review a squashed and rebased
>>> set of
>>>>> commits than if it was if I could just review the changes in a commit.
>>> Does
>>>>> anyone object to me adding in the contributors guide something to the
>>>>> extent of "Although you may be asked to rebase or squash your
>>> contribution
>>>>> as part of the review process, don't feel the need to do so
>>> speculatively.
>>>>> The committer working on merging the contribution may prefer to do these
>>>>> types of operations as part of the merge process, and the history of
>>> your
>>>>> patch or pull request may aid in the review process"
>>>>> 
>>>>> Tony
>>> 


Mime
View raw message