[Pulp-dev] Disabling merge by commit

Fri Sep 25 15:14:58 UTC 2020

On Fri, Sep 25, 2020 at 4:48 PM Brian Bouterse <bmbouter at redhat.com> wrote:

>
>
> On Fri, Sep 25, 2020 at 8:14 AM Matthias Dellweg <mdellweg at redhat.com>
> wrote:
>
>> I know, this is late to the game, but i didn't have a better argument
>> than i didn't like. Now i know why i prefer merge commits:
>> Having proper merge commits:
>> - keeps the information when a change was made and when it was merged
>> (quba42 mentioned this before)
>> - keeps signatures on commits intakt (I know almost no one signes their
>> commits, i do)
>>
> I never thought of this, but throwing away signed commits is kind of a big
> deal. I love the rebase default, and don't like the merge commit option,
> but this is a deal-breaker for me. Over time I think we want more signed
> commits for project security.
>

More of a feeling again, but i feel uneasy in general that github is
changing / rewriting my commits.

>
> - allows to delete your feature branches with `git branch -d` instead of
>> `git branch -D` and thereby prevents unintentional loss of unmerged work
>>
>> The most prominent argument i heard here was for a clean git history.
>> For me a clean git history should focus on the individual commit, like:
>> - one commit for one change
>> - good commit messages (https://chris.beams.io/posts/git-commit/ is a
>> very good guideline)
>> - keep the diff short and readable (black does a good job there with
>> trailing commas and no hanging indents)
>>
>> For this last bullet point, i want to suggest, that we drop the "Break
>> documentation lines at 100 characters and try to imitate block formatting."
>> rule.
>> We should break lines in rst and md files like we do in all other source
>> code at language structures. If you have one sentence per line, and you
>> want to change one sentence, the resulting diff reflects just that (+1-1);
>> If you delete a phrase (-1); If you add two sentences (+2). This also helps
>> to find the commit, a specific part of the docs was changed for real via
>> "git blame".
>>
>> On Fri, Sep 25, 2020 at 1:11 AM David Davis <daviddavis at redhat.com>
>> wrote:
>>
>>> I agree that merging by squash is potentially unsafe. I'll disable it
>>> for pulpcore and pulp_file unless anyone objects.
>>>
>>> David
>>>
>>>
>>> On Thu, Sep 24, 2020 at 11:56 AM Brian Bouterse <bmbouter at redhat.com>
>>> wrote:
>>>
>>>> I'm in favor of only the rebase & merge option everywhere. Our commit
>>>> association machinery relies on commits not being modified, so I don't
>>>> think the "squash and rebase" is a safe option for us. I am glad we are no
>>>> longer using merge commits also.
>>>>
>>>> On Thu, Sep 24, 2020 at 11:39 AM Tatiana Tereshchenko <
>>>> ttereshc at redhat.com> wrote:
>>>>
>>>>> pulp_rpm left only rebase & merge option.
>>>>>
>>>>> On Wed, Sep 23, 2020 at 7:46 PM Mike DePaulo <mikedep333 at redhat.com>
>>>>> wrote:
>>>>>
>>>>>> On Wed, Sep 23, 2020 at 9:52 AM Justin Sherrill <jsherril at redhat.com>
>>>>>> wrote:
>>>>>>
>>>>>>>
>>>>>>> On 9/23/20 7:18 AM, David Davis wrote:
>>>>>>>
>>>>>>> I think the two main things for me are (1) it makes git history more
>>>>>>> linear and (2) it cuts down on the number of commits. Both of these make
>>>>>>> git history more readable.
>>>>>>>
>>>>>>> 3rd main thing for me:
>>>>>> 3. It removes extra work when rewriting history. Rewriting history is
>>>>>> desirable in case secret keys, huge binary blobs (that degrade git
>>>>>> performance), etc accidentally get through.
>>>>>>
>>>>>>> The 'rebase and merge' option provides a nice balance of letting you
>>>>>>> provide multiple commits and maintain commit history while not creating a
>>>>>>> merge commit and  making a hard to read commit history.  Sometimes it is
>>>>>>> more expressive to have two (or three) commits that make up one pr to make
>>>>>>> it into the source tree.
>>>>>>>
>>>>>> I agree with rebase and merge. Often I need multiple commits for that
>>>>>> reason, or for multiple closely related (pulp_installer) tickets.
>>>>>>
>>>>>> I've done this both on the X2Go Project
>>>>>> <https://wiki.x2go.org/doku.php>, and at a previous job with a big
>>>>>> ansible codebase.
>>>>>>
>>>>>> -Mike
>>>>>>
>>>>>>
>>>>>> David
>>>>>>
>>>>>>
>>>>>> On Wed, Sep 23, 2020 at 6:48 AM Ina Panova <ipanova at redhat.com>
>>>>>> wrote:
>>>>>>
>>>>>>> Hi Quirin,
>>>>>>>
>>>>>>>
>>>>>>> --------
>>>>>>> Regards,
>>>>>>>
>>>>>>> Ina Panova
>>>>>>> Senior Software Engineer| Pulp| Red Hat Inc.
>>>>>>>
>>>>>>> "Do not go where the path may lead,
>>>>>>>  go instead where there is no path and leave a trail."
>>>>>>>
>>>>>>>
>>>>>>> On Wed, Sep 23, 2020 at 10:47 AM Quirin Pamp <pamp at atix.de> wrote:
>>>>>>>
>>>>>>>> "I'd encourage plugins to consider disabling merge by commit as
>>>>>>>> well."
>>>>>>>>
>>>>>>>> In order to evaluate this it would be great, if you could explain
>>>>>>>> why this was decided for pulpcore and pulp_file.
>>>>>>>> You have posted a lot of general information about the different
>>>>>>>> merge  type (the "What?"), but not so much on the "Why?".
>>>>>>>>
>>>>>>>> As far as I can tell the main advantage of squish and rebase, is
>>>>>>>> that it leads to a more tidy history in master, at the cost of losing some
>>>>>>>> information on how the sausage was made.
>>>>>>>> As a result squish and rebase becomes increasingly advantageous
>>>>>>>> with increasing PR volume.
>>>>>>>> However, I fail to see an advantage for pulp_deb, which does not
>>>>>>>> have a large PR volume.
>>>>>>>>
>>>>>>>> Or am I missing some relevant part of the argument?
>>>>>>>>
>>>>>>>
>>>>>>> I think your understanding is correct. In my perspective it is
>>>>>>> important to have a tidy history in master no matter how high/low PR
>>>>>>> traffic you have.
>>>>>>>
>>>>>>> pulp_container has disabled merge by commit as well.
>>>>>>>
>>>>>>>>
>>>>>>>> Quirin
>>>>>>>> ------------------------------
>>>>>>>> *From:* pulp-dev-bounces at redhat.com <pulp-dev-bounces at redhat.com>
>>>>>>>> on behalf of David Davis <daviddavis at redhat.com>
>>>>>>>> *Sent:* 22 September 2020 17:16
>>>>>>>> *To:* Pulp-dev <pulp-dev at redhat.com>
>>>>>>>> *Subject:* Re: [Pulp-dev] Disabling merge by commit
>>>>>>>>
>>>>>>>> Here's some more information about PR merges as well:
>>>>>>>>
>>>>>>>>
>>>>>>>> https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/about-pull-request-merges
>>>>>>>>
>>>>>>>> David
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Sep 22, 2020 at 11:11 AM David Davis <daviddavis at redhat.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Today at open floor, we decided to disable merging by commit for
>>>>>>>> pulpcore and pulp_file PRs. Instead, developers will rebase or squash PRs
>>>>>>>> to merge them. This adds the changes to HEAD instead of
>>>>>>>> interspersing commits and creating a merge commit. This picture of git
>>>>>>>> history comparing pulpcore to foreman (which doesn't merge by commit)
>>>>>>>> illustrates the differences:
>>>>>>>>
>>>>>>>> https://imgur.com/a/uiIa0Mr
>>>>>>>>
>>>>>>>> I'd encourage plugins to consider disabling merge by commit as
>>>>>>>> well. To do so, go to the settings page for your github repo and look
>>>>>>>> under the Merge Button section.
>>>>>>>>
>>>>>>>> David
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> Pulp-dev mailing list
>>>>>>>> Pulp-dev at redhat.com
>>>>>>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>>>>>>
>>>>>>>
>>>>>> _______________________________________________
>>>>>> Pulp-dev mailing listPulp-dev at redhat.comhttps://www.redhat.com/mailman/listinfo/pulp-dev
>>>>>>
>>>>>> _______________________________________________
>>>>>>> Pulp-dev mailing list
>>>>>>> Pulp-dev at redhat.com
>>>>>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>>
>>>>>> Mike DePaulo
>>>>>>
>>>>>> He / Him / His
>>>>>>
>>>>>> Service Reliability Engineer, Pulp
>>>>>>
>>>>>> Red Hat <https://www.redhat.com/>
>>>>>>
>>>>>> IM: mikedep333
>>>>>>
>>>>>> GPG: 51745404
>>>>>> <https://www.redhat.com/>
>>>>>> _______________________________________________
>>>>>> Pulp-dev mailing list
>>>>>> Pulp-dev at redhat.com
>>>>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>>>>
>>>>> _______________________________________________
>>>>> Pulp-dev mailing list
>>>>> Pulp-dev at redhat.com
>>>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>>>
>>>> _______________________________________________
>>>> Pulp-dev mailing list
>>>> Pulp-dev at redhat.com
>>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>>
>>> _______________________________________________
>>> Pulp-dev mailing list
>>> Pulp-dev at redhat.com
>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>
>> _______________________________________________
>> Pulp-dev mailing list
>> Pulp-dev at redhat.com
>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20200925/b62ed61b/attachment.htm>