[Pulp-dev] Disabling merge by commit

David Davis daviddavis at redhat.com
Fri Sep 25 16:28:17 UTC 2020


I think it's worth asking about what problems we are trying to solve. I
don't think that preserving signed commits is that useful unless we are
trying to maximize our level of security by requiring all commits to be
signed (and I question if we'd actually want or need this). If we do want
to preserve signed commits but also allow unsigned commits, then I'm
wondering what problem are we trying to solve?

One other thing I want to point out is that no information is lost. If you
look at any commit against master on Github (whether it was rebased or
not), you can see the PR which it came from which will tell you who created
it, when the commit was made, if it was signed, etc.

David


On Fri, Sep 25, 2020 at 11:15 AM Matthias Dellweg <mdellweg at redhat.com>
wrote:

>
>
> 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/1009017c/attachment.htm>


More information about the Pulp-dev mailing list