You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@arrow.apache.org by Jacob Wujciak <ja...@voltrondata.com.INVALID> on 2023/02/01 02:46:24 UTC
Re: [DISCUSS] The default commit message for merge button
Yes same here I personally don't use the git log much but I am fine with
title + description if it is beneficial for others
Joris Van den Bossche <jo...@gmail.com> schrieb am Di., 31.
Jan. 2023, 22:28:
> And to be explicit, as mentioned in my original post, I am fine with
> both, so +1 on title + description
>
> On Tue, 31 Jan 2023 at 21:53, Matthew Topol
> <ma...@voltrondata.com.invalid> wrote:
> >
> > (non-binding) I am also +1 for title + description....
> >
> > On Tue, Jan 31, 2023 at 11:17 AM Neal Richardson <
> > neal.p.richardson@gmail.com> wrote:
> >
> > > +1, with or without description
> > >
> > > Neal
> > >
> > > On Tue, Jan 31, 2023 at 11:04 AM Weston Pace <we...@gmail.com>
> > > wrote:
> > >
> > > > +1 to both. This only applies to the merge button and, in that
> case, the
> > > > committer has a chance to review the message before merging. So if
> there
> > > > is garbage in the description hopefully they can catch this here and
> > > adjust
> > > > it or ask for a cleanup of the description.
> > > >
> > > > Also, it seems we're trying to be more "conventional"[1] with things
> like
> > > > BREAKING CHANGE in the description. If we want to go that route the
> > > > description is necessary.
> > > >
> > > > [1] https://www.conventionalcommits.org/en/v1.0.0/
> > > >
> > > > On Tue, Jan 31, 2023, 5:49 AM David Li <li...@apache.org> wrote:
> > > >
> > > > > I am also +1 for title + description.
> > > > >
> > > > > On Tue, Jan 31, 2023, at 05:16, Felipe Oliveira Carvalho wrote:
> > > > > > +1 for "pull request title *and* description".
> > > > > >
> > > > > > Being able to read descriptions without leaving the editor is
> handy.
> > > > > > Keeping that information tracked in the repo means we don’t
> depend on
> > > > > > GitHub to reconstruct the history of the project.
> > > > > >
> > > > > > On Tue, 31 Jan 2023 at 06:43 Antoine Pitrou <an...@python.org>
> > > > wrote:
> > > > > >
> > > > > >>
> > > > > >> +1 for "pull request title *and* description".
> > > > > >>
> > > > > >> I'd rather have the description recorded in git than have to
> look
> > > up a
> > > > > >> PR to get more explanations. Also, we don't know what Github
> will
> > > have
> > > > > >> become in 10 years.
> > > > > >>
> > > > > >>
> > > > > >>
> > > > > >> Le 31/01/2023 à 09:53, Joris Van den Bossche a écrit :
> > > > > >> > I would personally prefer to use just "Pull request title"
> instead
> > > > of
> > > > > >> > "Pull request title and description".
> > > > > >> >
> > > > > >> > In my experience, including the description in the commit
> message
> > > > (as
> > > > > >> > we already do) more often gives noise to the output of `git
> log`,
> > > > and
> > > > > >> > you can always go from the commit to the PR to see the full
> > > context.
> > > > > >> > In many cases, the description is quite verbose or contain
> long
> > > > > >> > examples, or might be outdated (written when the PR was
> opened,
> > > but
> > > > > >> > the PR might have changed along the review process), ...
> > > Especially
> > > > > >> > now that we have the github PR template with sections, they
> might
> > > > > >> > become even more verbose.
> > > > > >> >
> > > > > >> > Personally, when opening a PR myself, I often leave the top
> post
> > > > > >> > empty, to add a second comment with more explanation, exactly
> to
> > > > avoid
> > > > > >> > including that in the commit message if that doesn't seem
> useful
> > > to
> > > > > >> > me.
> > > > > >> >
> > > > > >> > Anyway, I am certainly OK with both options if the general
> > > consensus
> > > > > >> > is for "Pull request title and description" (and certainly if
> that
> > > > > >> > enables actually using the merge button), but just stating my
> > > > personal
> > > > > >> > preference.
> > > > > >> >
> > > > > >> > On Tue, 31 Jan 2023 at 09:33, Raúl Cumplido <
> > > raulcumplido@gmail.com
> > > > >
> > > > > >> wrote:
> > > > > >> >>
> > > > > >> >> +1
> > > > > >> >> We already do it on the merge script and we have already
> changed
> > > it
> > > > > on
> > > > > >> the
> > > > > >> >> `arrow-site` repo.
> > > > > >> >>
> > > > > >> >> El mar, 31 ene 2023 a las 9:13, Sutou Kouhei (<
> > > kou@clear-code.com
> > > > >)
> > > > > >> >> escribió:
> > > > > >> >>
> > > > > >> >>> Hi,
> > > > > >> >>>
> > > > > >> >>> We need to get consensus to change the default commit
> > > > > >> >>> message for merge button:
> > > > > >> >>>
> > > > > >> >>> https://issues.apache.org/jira/browse/INFRA-24133
> > > > > >> >>>
> > > > > >> >>>> Could you change the default commit message when merging a
> > > > > >> >>>> PR to "Default to pull request title and description" on
> > > > > >> >>>> the following Apache Arrow related repositories?
> > > > > >> >>>>
> > > > > >> >>>> * https://github.com/apache/arrow
> > > > > >> >>>> * https://github.com/apache/arrow-adbc
> > > > > >> >>>> * https://github.com/apache/arrow-cookbook
> > > > > >> >>>> * https://github.com/apache/arrow-flight-sql-postgresql
> > > > > >> >>>> * https://github.com/apache/arrow-julia
> > > > > >> >>>> * https://github.com/apache/arrow-nanoarrow
> > > > > >> >>>> * https://github.com/apache/arrow-testing
> > > > > >> >>>>
> > > > > >> >>>> See also:
> > > > > >> >>>>
> > > > > >> >>>
> > > > > >>
> > > > >
> > > >
> > >
> https://github.blog/changelog/2022-08-23-new-options-for-controlling-the-default-commit-message-when-merging-a-pull-request/
> > > > > >> >>>>
> > > > > >> >>>> Related: https://issues.apache.org/jira/browse/INFRA-24122
> > > > > >> >>>
> > > > > >> >>>
> > > > https://issues.apache.org/jira/browse/INFRA-24133#comment-17682383
> > > > > >> >>>
> > > > > >> >>>> Please provide a mailing list link to project consensus on
> this
> > > > > >> change.
> > > > > >> >>>
> > > > > >> >>>
> > > > > >> >>> How about changing the default commit message for merge
> > > > > >> >>> button to "Default to pull request title and description"
> > > > > >> >>> like our dev/merge_arrow_pr.py in apache/arrow does?
> > > > > >> >>>
> > > > > >> >>> Note that this doesn't mean that we drop
> > > > > >> >>> dev/merge_arrow_pr.py immediately. It's a separated
> > > > > >> >>> discussion.
> > > > > >> >>>
> > > > > >> >>>
> > > > > >> >>> Thanks,
> > > > > >> >>> --
> > > > > >> >>> kou
> > > > > >> >>>
> > > > > >>
> > > > >
> > > >
> > >
>
Re: [DISCUSS] The default commit message for merge button
Posted by Yibo Cai <yi...@arm.com>.
+1 for title and description
For purely personal reason. I care about commit messages and often try
writing informative messages (even following pedantic rules like 72
chars length). It's a bit disappointed if the messages are not shown by
`git log`.
On 2/1/23 10:46, Jacob Wujciak wrote:
> Yes same here I personally don't use the git log much but I am fine with
> title + description if it is beneficial for others
>
> Joris Van den Bossche <jo...@gmail.com> schrieb am Di., 31.
> Jan. 2023, 22:28:
>
>> And to be explicit, as mentioned in my original post, I am fine with
>> both, so +1 on title + description
>>
>> On Tue, 31 Jan 2023 at 21:53, Matthew Topol
>> <ma...@voltrondata.com.invalid> wrote:
>>>
>>> (non-binding) I am also +1 for title + description....
>>>
>>> On Tue, Jan 31, 2023 at 11:17 AM Neal Richardson <
>>> neal.p.richardson@gmail.com> wrote:
>>>
>>>> +1, with or without description
>>>>
>>>> Neal
>>>>
>>>> On Tue, Jan 31, 2023 at 11:04 AM Weston Pace <we...@gmail.com>
>>>> wrote:
>>>>
>>>>> +1 to both. This only applies to the merge button and, in that
>> case, the
>>>>> committer has a chance to review the message before merging. So if
>> there
>>>>> is garbage in the description hopefully they can catch this here and
>>>> adjust
>>>>> it or ask for a cleanup of the description.
>>>>>
>>>>> Also, it seems we're trying to be more "conventional"[1] with things
>> like
>>>>> BREAKING CHANGE in the description. If we want to go that route the
>>>>> description is necessary.
>>>>>
>>>>> [1] https://www.conventionalcommits.org/en/v1.0.0/
>>>>>
>>>>> On Tue, Jan 31, 2023, 5:49 AM David Li <li...@apache.org> wrote:
>>>>>
>>>>>> I am also +1 for title + description.
>>>>>>
>>>>>> On Tue, Jan 31, 2023, at 05:16, Felipe Oliveira Carvalho wrote:
>>>>>>> +1 for "pull request title *and* description".
>>>>>>>
>>>>>>> Being able to read descriptions without leaving the editor is
>> handy.
>>>>>>> Keeping that information tracked in the repo means we don’t
>> depend on
>>>>>>> GitHub to reconstruct the history of the project.
>>>>>>>
>>>>>>> On Tue, 31 Jan 2023 at 06:43 Antoine Pitrou <an...@python.org>
>>>>> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> +1 for "pull request title *and* description".
>>>>>>>>
>>>>>>>> I'd rather have the description recorded in git than have to
>> look
>>>> up a
>>>>>>>> PR to get more explanations. Also, we don't know what Github
>> will
>>>> have
>>>>>>>> become in 10 years.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Le 31/01/2023 à 09:53, Joris Van den Bossche a écrit :
>>>>>>>>> I would personally prefer to use just "Pull request title"
>> instead
>>>>> of
>>>>>>>>> "Pull request title and description".
>>>>>>>>>
>>>>>>>>> In my experience, including the description in the commit
>> message
>>>>> (as
>>>>>>>>> we already do) more often gives noise to the output of `git
>> log`,
>>>>> and
>>>>>>>>> you can always go from the commit to the PR to see the full
>>>> context.
>>>>>>>>> In many cases, the description is quite verbose or contain
>> long
>>>>>>>>> examples, or might be outdated (written when the PR was
>> opened,
>>>> but
>>>>>>>>> the PR might have changed along the review process), ...
>>>> Especially
>>>>>>>>> now that we have the github PR template with sections, they
>> might
>>>>>>>>> become even more verbose.
>>>>>>>>>
>>>>>>>>> Personally, when opening a PR myself, I often leave the top
>> post
>>>>>>>>> empty, to add a second comment with more explanation, exactly
>> to
>>>>> avoid
>>>>>>>>> including that in the commit message if that doesn't seem
>> useful
>>>> to
>>>>>>>>> me.
>>>>>>>>>
>>>>>>>>> Anyway, I am certainly OK with both options if the general
>>>> consensus
>>>>>>>>> is for "Pull request title and description" (and certainly if
>> that
>>>>>>>>> enables actually using the merge button), but just stating my
>>>>> personal
>>>>>>>>> preference.
>>>>>>>>>
>>>>>>>>> On Tue, 31 Jan 2023 at 09:33, Raúl Cumplido <
>>>> raulcumplido@gmail.com
>>>>>>
>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> +1
>>>>>>>>>> We already do it on the merge script and we have already
>> changed
>>>> it
>>>>>> on
>>>>>>>> the
>>>>>>>>>> `arrow-site` repo.
>>>>>>>>>>
>>>>>>>>>> El mar, 31 ene 2023 a las 9:13, Sutou Kouhei (<
>>>> kou@clear-code.com
>>>>>> )
>>>>>>>>>> escribió:
>>>>>>>>>>
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> We need to get consensus to change the default commit
>>>>>>>>>>> message for merge button:
>>>>>>>>>>>
>>>>>>>>>>> https://issues.apache.org/jira/browse/INFRA-24133
>>>>>>>>>>>
>>>>>>>>>>>> Could you change the default commit message when merging a
>>>>>>>>>>>> PR to "Default to pull request title and description" on
>>>>>>>>>>>> the following Apache Arrow related repositories?
>>>>>>>>>>>>
>>>>>>>>>>>> * https://github.com/apache/arrow
>>>>>>>>>>>> * https://github.com/apache/arrow-adbc
>>>>>>>>>>>> * https://github.com/apache/arrow-cookbook
>>>>>>>>>>>> * https://github.com/apache/arrow-flight-sql-postgresql
>>>>>>>>>>>> * https://github.com/apache/arrow-julia
>>>>>>>>>>>> * https://github.com/apache/arrow-nanoarrow
>>>>>>>>>>>> * https://github.com/apache/arrow-testing
>>>>>>>>>>>>
>>>>>>>>>>>> See also:
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>
>>>>
>> https://github.blog/changelog/2022-08-23-new-options-for-controlling-the-default-commit-message-when-merging-a-pull-request/
>>>>>>>>>>>>
>>>>>>>>>>>> Related: https://issues.apache.org/jira/browse/INFRA-24122
>>>>>>>>>>>
>>>>>>>>>>>
>>>>> https://issues.apache.org/jira/browse/INFRA-24133#comment-17682383
>>>>>>>>>>>
>>>>>>>>>>>> Please provide a mailing list link to project consensus on
>> this
>>>>>>>> change.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> How about changing the default commit message for merge
>>>>>>>>>>> button to "Default to pull request title and description"
>>>>>>>>>>> like our dev/merge_arrow_pr.py in apache/arrow does?
>>>>>>>>>>>
>>>>>>>>>>> Note that this doesn't mean that we drop
>>>>>>>>>>> dev/merge_arrow_pr.py immediately. It's a separated
>>>>>>>>>>> discussion.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> --
>>>>>>>>>>> kou
>>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>
>>>>
>>
>