[Pulp-dev] PUP-3: Proposal to change our git workflow

Mon Jun 12 17:03:34 UTC 2017

Yes, it's more of a guide than an absolute rule. If a merging developer
wants to cherry-pick and resolve conflicts, we should not disallow that. To
capture that aspect, the current language could be reverted back to the "we
should consider..." language that would allow the merger to have discretion
and choice.

I'm +1 still in both cases.

As an aside, reviewing a merge conflict resolution is not easy in git. I'm
stating this hoping I just don't understand it and that someone can show me
how it's done some other time. After spending a good amount of time in #git
one day on this topic w/ @asmacdo I'm convinced this isn't a use case git
handles well. If that is true then having a review process for conflict
resolution is something git and github won't allow us to facilitate. I hope
I'm wrong about this.

-Brian

On Mon, Jun 12, 2017 at 10:01 AM, David Davis <daviddavis at redhat.com> wrote:

> Regarding your concern about not cherry-picking if there are conflicts,
> the PUP originally said “we should consider …”. I think we went with
> stronger language though to dissuade developers from cherry-picking (by
> default) if there are conflicts. That said, as in the use case you mention,
> I think that’s a perfectly fine example of when it’s ok to cherry-pick even
> though there’s a conflict. I think of this as more of a guideline than an
> absolute rule.
>
>
> David
>
> On Mon, Jun 12, 2017 at 9:23 AM, Michael Hrivnak <mhrivnak at redhat.com>
> wrote:
>
>> -0 for similar reasons to those already discussed.
>>
>> We've identified alternatives that would address many of the concerns
>> listed in the Motivation section. To re-cap:
>>
>> Merge mistakes: we have multiple options we could implement with the
>> merge-forward option.
>>
>> Community PR rebasing: we can either use the new github feature to
>> auto-rebase, or take the same approach as the PUP (let the PR stay on
>> master) and just accept that some of those changes won't make it into a Z
>> release.
>>
>> Merge conflict resolution: the PUP says this can lead to mistakes that go
>> unreviewed, but we've also heard feedback in this discussion that merging
>> forward is almost never a problem. My view is that on rare occasions a
>> merge conflict can get resolved incorrectly, but it's not a substantial
>> problem for us, and those cases should be easily caught with automated
>> testing. We will make mistakes that break master for lots of reasons, some
>> that even go unnoticed in review. We should strive to make those rare, but
>> also should have confidence that our testing will catch such problems and
>> enable us to quickly correct them. Our current guidelines do encourage
>> review for merge conflicts that are substantial:
>>
>> http://docs.pulpproject.org/dev-guide/contributing/merging.
>> html#merging-your-changes
>>
>> We could make the language around that stronger, or even require review
>> for such cases.
>>
>> All of that said, while I think we have ways to address nearly all the
>> PUP's concerns without changing to a cherry-pick model, I'm certainly
>> willing to try such a model. My preference would be to try some
>> less-drastic changes to our process first, and see how that goes. But I am
>> sure that we could be successful with a cherry-pick model, and given broad
>> interest am willing to try it.
>>
>> I have only one big concern: "If the cherry-pick has conflicts, we should
>> abandon the cherry-pick and encourage users to upgrade to the next Y
>> release." I worry that this will have a big impact on our ability to
>> deliver bug fixes. As long as we retain some discretion to employ a small
>> amount of conflict resolution when reasonable, I think this could be a fine
>> guideline. But if we were to apply it as an absolute rule, it may affect
>> many more changes, and substantially slow down our ability to release fixes
>> early and often. For example, often when we see a conflict, it is with
>> import statements. It would be a shame to omit a fix from a Z release over
>> that.
>>
>> On Fri, Jun 9, 2017 at 9:09 AM, David Davis <daviddavis at redhat.com>
>> wrote:
>>
>>> Just wanted to send out a reminder that voting is ending on Monday, June
>>> 12 at 9pm UTC. I haven’t seen much of a response around trying to adopt an
>>> alternative to PUP-3 that doesn’t involve cherry-picking so I am going to
>>> assume there isn’t much interest in doing so. Again, please respond with
>>> any thoughts or feel free to change your vote if that’s not the case.
>>> Thanks.
>>>
>>>
>>> David
>>>
>>> On Tue, Jun 6, 2017 at 4:59 PM, David Davis <daviddavis at redhat.com>
>>> wrote:
>>>
>>>> Talking with @bmbouter a little more about the PUP process and looking
>>>> back at PUP-1, I think that the only way for PUP-3 to not be accepted is if
>>>> a core developer were to cast/recast a -1 vote. I know there has been talk
>>>> about alternatives but looking at the votes, there is a consensus around
>>>> adopting PUP-3:
>>>>
>>>> +1 - 5 votes
>>>> +0 - 1 vote
>>>> -0 - 2 votes
>>>> -1 - 0 votes
>>>>
>>>> If anyone feels strongly about trying out an alternative or discussing
>>>> alternatives further, please recast your vote or respond with your
>>>> concerns. Otherwise, I think we'll proceed with approving/rejecting the PUP
>>>> based on the votes on the deadline of June 12th.
>>>>
>>>>
>>>> David
>>>>
>>>> On Tue, Jun 6, 2017 at 4:27 PM, David Davis <daviddavis at redhat.com>
>>>> wrote:
>>>>
>>>>> I have updated the proposal’s motivation section. Note that the actual
>>>>> change/workflow hasn’t changed at all.
>>>>>
>>>>>
>>>>> David
>>>>>
>>>>> On Tue, Jun 6, 2017 at 4:08 PM, David Davis <daviddavis at redhat.com>
>>>>> wrote:
>>>>>
>>>>>> Looks like @bmbouter made a comment to include this but I forgot to
>>>>>> include it:
>>>>>>
>>>>>> https://github.com/pulp/pups/pull/3#discussion_r111498031
>>>>>>
>>>>>> Will update the PUP.
>>>>>>
>>>>>>
>>>>>> David
>>>>>>
>>>>>> On Tue, Jun 6, 2017 at 3:48 PM, Michael Hrivnak <mhrivnak at redhat.com>
>>>>>> wrote:
>>>>>>
>>>>>>>
>>>>>>> On Tue, Jun 6, 2017 at 9:58 AM, Brian Bouterse <bbouters at redhat.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> I think we need to redo the git workflow because we can't continue
>>>>>>>> to resolve conflicts during merge forward as we did before. I see that as
>>>>>>>> the central issue the PUP is resolving.
>>>>>>>
>>>>>>>
>>>>>>> The PUP likely needs additional revision in that case; it does not
>>>>>>> mention conflict resolution at all as a motivation. It would be valuable to
>>>>>>> spell that out and discuss it.
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>>
>>>>>>> Michael Hrivnak
>>>>>>>
>>>>>>> Principal Software Engineer, RHCE
>>>>>>>
>>>>>>> Red Hat
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>>
>> --
>>
>> Michael Hrivnak
>>
>> Principal Software Engineer, RHCE
>>
>> Red Hat
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20170612/d8c18798/attachment.htm>