You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by julio cesar sanchez <jc...@gmail.com> on 2019/10/01 11:18:03 UTC
vote: PR merge convention
Last year, Jan started a thread with different topics and one of them was
to have a merge convention. I copy the text:
> 3. Merge Conventions / Protected Branch:
> Connected to all that is my suggestion to protect the `master` branch so
that by default nobody can commit there - all changes have to be made via
Pull Requests. Pull Requests are by default merged via the "Squash + Merge"
button / functionality so that all commits are squashed into one clean
commit per change. This also enforces the commit message structure I posted
above. (Of course committers can choose to _not_ use Squash + Merge if
appropriate for the PR - e.g. when cherry picking commits from a release
branch or similar).
> What do you think about this suggestion?
Looks like we didn't agree on anything, but can we agree now?
I've checked a few repos and some of them have a lot of commits from the
same PR with meaningless commit messages when changes were requested, plus
the ugly "merge PR ### from YYY" that makes the commit history hard to
follow and hard to cherry pick if needed.
Since I'm not sure if we can protect branches, I'll focus only on the merge
convention.
Can we all agree on using the "squash + merge" for user PRs, unless we
think the different commits makes sense, in this case we should try the
"rebase and merge" button.
I vote +1
Re: vote: PR merge convention
Posted by julio cesar sanchez <jc...@gmail.com>.
It’s not just you Jesse, there are a few in a few repos from different
people. Didn’t mean to attack you, just wanted to remind it as we all agreed
El El sáb, 2 may 2020 a las 20:41, Jesse <pu...@gmail.com> escribió:
> I made a mistake, no need to create laws or rules.
>
> > On May 2, 2020, at 11:21 AM, Tim Brust <ti...@gmail.com> wrote:
> >
> > +1 to Niklas suggestion :)
> >
> > Sent from my iPhone
> >
> >>> On 2. May 2020, at 6:54 PM, Niklas Merz <ni...@apache.org> wrote:
> >>>
> >>> We could try to enforce this setting with .asf.yml. I saw this posted
> on another list.
> >>>
> >>> See:
> https://cwiki.apache.org/confluence/display/INFRA/.asf.yaml+features+for+git+repositories#id-.asf.yamlfeaturesforgitrepositories-Mergebuttons
> >>>
> >>> Should we roll this out to all repos?
> >>>
> >>> May 2, 2020 1:38 PM, "julio cesar sanchez" <jc...@gmail.com>
> wrote:
> >>>
> >>> Just a reminder that we agreed on using the squash and merge, but I
> still
> >>> see regular merge commits in a few repos from time to time.
> >>>
> >>>> El El sáb, 5 oct 2019 a las 19:32, gandhi rajan <
> gandhirajan.n@gmail.com>
> >>>> escribió:
> >>>>
> >>>> + 1 for squash and merge as it makes the repo history cleaner.
> >>>>
> >>>>> On Sat, Oct 5, 2019 at 8:34 PM <ra...@gmail.com> wrote:
> >>>>
> >>>> +1 for "Squash and merge" as the default strategy
> >>>>
> >>>> Regarding "Create a merge commit":
> >>>> At times, I find this option valuable too. Consider a PR that cleans
> up a
> >>>> test suite. As part of that cleanup I might re-order the test cases.
> As
> >>>> re-ordering produces a noisy diff, I usually isolate it in its own
> >>>> commit.
> >>>> I would not want to merge this commit with the others as that would
> lead
> >>>> to
> >>>> a commit with a very incomprehensible diff. So in this case I would
> favor
> >>>> leaving the commits separate and doing an actual merge to group them
> >>>> together in the global history.
> >>>>
> >>>> Am Fr., 4. Okt. 2019 um 17:03 Uhr schrieb julio cesar sanchez <
> >>>> jcesarmobile@gmail.com>:
> >>>>
> >>>>> Sorry if it wasn't clear, I said I was leaving the protecting master
> >>>> branch
> >>>>> out of the vote.
> >>>>>
> >>>>> Looks like we all agree on using "Squash and merge" as default,
> unless
> >>>> it
> >>>>> makes more sense to use one of the other options.
> >>>>>
> >>>>> El jue., 3 oct. 2019 a las 3:43, Bryan Ellis (<er...@apache.org>)
> >>>>> escribió:
> >>>>>
> >>>>>> -1 for protected master branches.
> >>>>>> Protecting a branch is a great idea except it does not work with our
> >>>>>> current workflow process. COHO commits directly onto the master
> >>>> branch
> >>>>> for
> >>>>>> releases. We would have to spend more time planning and changing our
> >>>>> entire
> >>>>>> current workflow process to eliminate direct commits if we wanted to
> >>>>>> protect branches. There is alternative such keeping master open for
> >>>>> direct
> >>>>>> commits and development while creating a protected "production"
> >>>> branch.
> >>>>>> Anyways this part of the discussion is off-topic and could be
> another
> >>>>>> discussion... Anyways, stated above I am downvoting protected
> >>>> branches
> >>>>> for
> >>>>>> now.
> >>>>>>
> >>>>>> +1 for "Squash and merge"
> >>>>>> Makes a nice single commit with the PR number and without the extra
> >>>> merge
> >>>>>> commit.
> >>>>>>
> >>>>>> +1 for "Rebase and merge"
> >>>>>> There are use cases where this can work perfectly.
> >>>>>> For example, Cordova-Electron has a `draft-2.0.0` branch which is
> >>>>> targeting
> >>>>>> the next major release. Major PRs were merged into that branch with
> >>>>> "Squash
> >>>>>> and merge". This means all PRs have nice PR# information in the
> >>>> title.
> >>>>>> A PR will later be created to merge this branch onto master. "Rebase
> >>>> and
> >>>>>> merge" will be used and will not create the merge commit message and
> >>>> will
> >>>>>> not squash.
> >>>>>>
> >>>>>> -1 for "Create a merge commit"
> >>>>>> Not in favor of the extra merge commit. I think in most cases one PR
> >>>>> should
> >>>>>> focus on one task so that means it could be squashed when there are
> >>>>>> multiple commits. If "Create a merge commit" is used, each commit
> >>>> will
> >>>> be
> >>>>>> merged and does not contain the PR# in the title. When creating
> >>>> release
> >>>>>> notes, I need to manually review those commits to identify what PR
> it
> >>>>> came
> >>>>>> from to include the PR linking. Some times, depending on if they are
> >>>> all
> >>>>>> related commits, I need to manually group them and create a
> >>>> meaningful
> >>>>>> title for the release notes which I would prefer to have been done
> >>>>>> beforehand.
> >>>>>>
> >>>>>>
> >>>>>> On Wed, Oct 2, 2019 at 12:51 AM Jesse <pu...@gmail.com>
> >>>> wrote:
> >>>>>>
> >>>>>>> -1 for protected master branches, we are a small group of
> >>>> committers
> >>>>> and
> >>>>>>> don't need rules to keep us honest.
> >>>>>>> Protecting master would involve infra, as we cannot manage the
> >>>> minutia
> >>>>> in
> >>>>>>> github. I think we all do this anyway except for rare occasions.
> >>>>>>>
> >>>>>>> +1 for squash and merge, as long as the meaningful individual
> >>>> commit
> >>>>>>> messages get consolidated into the 1 commit.
> >>>>>>>
> >>>>>>> On Tue, Oct 1, 2019 at 8:36 AM Norman Breau <
> >>>> norman@normanbreau.com>
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>>> +1 to protect the master branch.
> >>>>>>>>
> >>>>>>>> Forcing PR will help organize commits if we need to go back in
> >>>>>>>> time to determine the reason why a change was made as the
> >>>>>>>> commit in github will show the corresponding PR. Which will
> >>>>>>>> (hopefully) be properly filled out with context and motivation,
> >>>>>>>> as well as the issues that the PR resolves.
> >>>>>>>>
> >>>>>>>> +1 for "squash + merge" as a default strategy.
> >>>>>>>>
> >>>>>>>> I feel like most of the time having all the individual commits
> >>>> that
> >>>>>>>> makes up a PR is not really necessary. Most of the time it's
> >>>>>>>> bloated with tweaks fixing errors that was introduced during the
> >>>>>>>> development of the PR or perhaps refactoring for code style, etc.
> >>>>>>>> If there are instances where squash shouldn't be used, that can
> >>>>>>>> be decided on a per-case basis in my opinion.
> >>>>>>>>
> >>>>>>>> On 2019-10-01 10:37 a.m., Chris Brody wrote:
> >>>>>>>>> I would agree that "squash + merge" should be used in *most*
> >>>> cases,
> >>>>>>>>> and I think there is no dispute on this point.
> >>>>>>>>>
> >>>>>>>>> In the few cases where there are too many things for a "squash
> >>>> +
> >>>>>>>>> merge" commit, and we have the changes down to a limited number
> >>>> of
> >>>>>>>>> clean, sensible commits, then I would favor merging with a
> >>>> merge
> >>>>>>>>> commit that shows the PR number.
> >>>>>>>>>
> >>>>>>>>> My issue with "rebase and merge" is that the commit history
> >>>> would
> >>>>> not
> >>>>>>>>> show the PR number.
> >>>>>>>>>
> >>>>>>>>> I think that having the commits show the PR number would make
> >>>> it
> >>>> a
> >>>>>>>>> little easier for whoever makes the release to make the release
> >>>>> notes
> >>>>>>>>> with the PR numbers.
> >>>>>>>>>
> >>>>>>>>> Are there any recent examples of "a lot of commits from the
> >>>> same
> >>>>> PR
> >>>>>>>>> with meaningless commit messages when changes were requested"?
> >>>>>>>>>
> >>>>>>>>> Maybe off-topic: I wonder if we should look for multiple
> >>>> committers
> >>>>>> to
> >>>>>>>>> approve an external contribution before merging?
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> On Tue, Oct 1, 2019 at 7:18 AM julio cesar sanchez
> >>>>>>>>> <jc...@gmail.com> wrote:
> >>>>>>>>>> Last year, Jan started a thread with different topics and one
> >>>> of
> >>>>>> them
> >>>>>>>> was
> >>>>>>>>>> to have a merge convention. I copy the text:
> >>>>>>>>>>
> >>>>>>>>>>> 3. Merge Conventions / Protected Branch:
> >>>>>>>>>>> Connected to all that is my suggestion to protect the
> >>>> `master`
> >>>>>> branch
> >>>>>>>> so
> >>>>>>>>>> that by default nobody can commit there - all changes have to
> >>>> be
> >>>>>> made
> >>>>>>>> via
> >>>>>>>>>> Pull Requests. Pull Requests are by default merged via the
> >>>>> "Squash +
> >>>>>>>> Merge"
> >>>>>>>>>> button / functionality so that all commits are squashed into
> >>>> one
> >>>>>> clean
> >>>>>>>>>> commit per change. This also enforces the commit message
> >>>>> structure I
> >>>>>>>> posted
> >>>>>>>>>> above. (Of course committers can choose to _not_ use Squash +
> >>>>> Merge
> >>>>>> if
> >>>>>>>>>> appropriate for the PR - e.g. when cherry picking commits
> >>>> from a
> >>>>>>> release
> >>>>>>>>>> branch or similar).
> >>>>>>>>>>
> >>>>>>>>>>> What do you think about this suggestion?
> >>>>>>>>>> Looks like we didn't agree on anything, but can we agree now?
> >>>>>>>>>>
> >>>>>>>>>> I've checked a few repos and some of them have a lot of
> >>>> commits
> >>>>> from
> >>>>>>> the
> >>>>>>>>>> same PR with meaningless commit messages when changes were
> >>>>>> requested,
> >>>>>>>> plus
> >>>>>>>>>> the ugly "merge PR ### from YYY" that makes the commit history
> >>>>> hard
> >>>>>> to
> >>>>>>>>>> follow and hard to cherry pick if needed.
> >>>>>>>>>>
> >>>>>>>>>> Since I'm not sure if we can protect branches, I'll focus only
> >>>> on
> >>>>>> the
> >>>>>>>> merge
> >>>>>>>>>> convention.
> >>>>>>>>>>
> >>>>>>>>>> Can we all agree on using the "squash + merge" for user PRs,
> >>>>> unless
> >>>>>> we
> >>>>>>>>>> think the different commits makes sense, in this case we
> >>>> should
> >>>>> try
> >>>>>>> the
> >>>>>>>>>> "rebase and merge" button.
> >>>>>>>>>>
> >>>>>>>>>> I vote +1
> >>>>>>>>>
> >>>>> ---------------------------------------------------------------------
> >>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> >>>>>>>>> For additional commands, e-mail: dev-help@cordova.apache.org
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>> ---------------------------------------------------------------------
> >>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> >>>>>>>> For additional commands, e-mail: dev-help@cordova.apache.org
> >>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>> --
> >>>> Regards,
> >>>> Gandhi
> >>>>
> >>>> "The best way to find urself is to lose urself in the service of
> others
> >>>> !!!"
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> >> For additional commands, e-mail: dev-help@cordova.apache.org
> >>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > For additional commands, e-mail: dev-help@cordova.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> For additional commands, e-mail: dev-help@cordova.apache.org
>
>
Re: vote: PR merge convention
Posted by Jesse <pu...@gmail.com>.
I made a mistake, no need to create laws or rules.
> On May 2, 2020, at 11:21 AM, Tim Brust <ti...@gmail.com> wrote:
>
> +1 to Niklas suggestion :)
>
> Sent from my iPhone
>
>>> On 2. May 2020, at 6:54 PM, Niklas Merz <ni...@apache.org> wrote:
>>>
>>> We could try to enforce this setting with .asf.yml. I saw this posted on another list.
>>>
>>> See: https://cwiki.apache.org/confluence/display/INFRA/.asf.yaml+features+for+git+repositories#id-.asf.yamlfeaturesforgitrepositories-Mergebuttons
>>>
>>> Should we roll this out to all repos?
>>>
>>> May 2, 2020 1:38 PM, "julio cesar sanchez" <jc...@gmail.com> wrote:
>>>
>>> Just a reminder that we agreed on using the squash and merge, but I still
>>> see regular merge commits in a few repos from time to time.
>>>
>>>> El El sáb, 5 oct 2019 a las 19:32, gandhi rajan <ga...@gmail.com>
>>>> escribió:
>>>>
>>>> + 1 for squash and merge as it makes the repo history cleaner.
>>>>
>>>>> On Sat, Oct 5, 2019 at 8:34 PM <ra...@gmail.com> wrote:
>>>>
>>>> +1 for "Squash and merge" as the default strategy
>>>>
>>>> Regarding "Create a merge commit":
>>>> At times, I find this option valuable too. Consider a PR that cleans up a
>>>> test suite. As part of that cleanup I might re-order the test cases. As
>>>> re-ordering produces a noisy diff, I usually isolate it in its own
>>>> commit.
>>>> I would not want to merge this commit with the others as that would lead
>>>> to
>>>> a commit with a very incomprehensible diff. So in this case I would favor
>>>> leaving the commits separate and doing an actual merge to group them
>>>> together in the global history.
>>>>
>>>> Am Fr., 4. Okt. 2019 um 17:03 Uhr schrieb julio cesar sanchez <
>>>> jcesarmobile@gmail.com>:
>>>>
>>>>> Sorry if it wasn't clear, I said I was leaving the protecting master
>>>> branch
>>>>> out of the vote.
>>>>>
>>>>> Looks like we all agree on using "Squash and merge" as default, unless
>>>> it
>>>>> makes more sense to use one of the other options.
>>>>>
>>>>> El jue., 3 oct. 2019 a las 3:43, Bryan Ellis (<er...@apache.org>)
>>>>> escribió:
>>>>>
>>>>>> -1 for protected master branches.
>>>>>> Protecting a branch is a great idea except it does not work with our
>>>>>> current workflow process. COHO commits directly onto the master
>>>> branch
>>>>> for
>>>>>> releases. We would have to spend more time planning and changing our
>>>>> entire
>>>>>> current workflow process to eliminate direct commits if we wanted to
>>>>>> protect branches. There is alternative such keeping master open for
>>>>> direct
>>>>>> commits and development while creating a protected "production"
>>>> branch.
>>>>>> Anyways this part of the discussion is off-topic and could be another
>>>>>> discussion... Anyways, stated above I am downvoting protected
>>>> branches
>>>>> for
>>>>>> now.
>>>>>>
>>>>>> +1 for "Squash and merge"
>>>>>> Makes a nice single commit with the PR number and without the extra
>>>> merge
>>>>>> commit.
>>>>>>
>>>>>> +1 for "Rebase and merge"
>>>>>> There are use cases where this can work perfectly.
>>>>>> For example, Cordova-Electron has a `draft-2.0.0` branch which is
>>>>> targeting
>>>>>> the next major release. Major PRs were merged into that branch with
>>>>> "Squash
>>>>>> and merge". This means all PRs have nice PR# information in the
>>>> title.
>>>>>> A PR will later be created to merge this branch onto master. "Rebase
>>>> and
>>>>>> merge" will be used and will not create the merge commit message and
>>>> will
>>>>>> not squash.
>>>>>>
>>>>>> -1 for "Create a merge commit"
>>>>>> Not in favor of the extra merge commit. I think in most cases one PR
>>>>> should
>>>>>> focus on one task so that means it could be squashed when there are
>>>>>> multiple commits. If "Create a merge commit" is used, each commit
>>>> will
>>>> be
>>>>>> merged and does not contain the PR# in the title. When creating
>>>> release
>>>>>> notes, I need to manually review those commits to identify what PR it
>>>>> came
>>>>>> from to include the PR linking. Some times, depending on if they are
>>>> all
>>>>>> related commits, I need to manually group them and create a
>>>> meaningful
>>>>>> title for the release notes which I would prefer to have been done
>>>>>> beforehand.
>>>>>>
>>>>>>
>>>>>> On Wed, Oct 2, 2019 at 12:51 AM Jesse <pu...@gmail.com>
>>>> wrote:
>>>>>>
>>>>>>> -1 for protected master branches, we are a small group of
>>>> committers
>>>>> and
>>>>>>> don't need rules to keep us honest.
>>>>>>> Protecting master would involve infra, as we cannot manage the
>>>> minutia
>>>>> in
>>>>>>> github. I think we all do this anyway except for rare occasions.
>>>>>>>
>>>>>>> +1 for squash and merge, as long as the meaningful individual
>>>> commit
>>>>>>> messages get consolidated into the 1 commit.
>>>>>>>
>>>>>>> On Tue, Oct 1, 2019 at 8:36 AM Norman Breau <
>>>> norman@normanbreau.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> +1 to protect the master branch.
>>>>>>>>
>>>>>>>> Forcing PR will help organize commits if we need to go back in
>>>>>>>> time to determine the reason why a change was made as the
>>>>>>>> commit in github will show the corresponding PR. Which will
>>>>>>>> (hopefully) be properly filled out with context and motivation,
>>>>>>>> as well as the issues that the PR resolves.
>>>>>>>>
>>>>>>>> +1 for "squash + merge" as a default strategy.
>>>>>>>>
>>>>>>>> I feel like most of the time having all the individual commits
>>>> that
>>>>>>>> makes up a PR is not really necessary. Most of the time it's
>>>>>>>> bloated with tweaks fixing errors that was introduced during the
>>>>>>>> development of the PR or perhaps refactoring for code style, etc.
>>>>>>>> If there are instances where squash shouldn't be used, that can
>>>>>>>> be decided on a per-case basis in my opinion.
>>>>>>>>
>>>>>>>> On 2019-10-01 10:37 a.m., Chris Brody wrote:
>>>>>>>>> I would agree that "squash + merge" should be used in *most*
>>>> cases,
>>>>>>>>> and I think there is no dispute on this point.
>>>>>>>>>
>>>>>>>>> In the few cases where there are too many things for a "squash
>>>> +
>>>>>>>>> merge" commit, and we have the changes down to a limited number
>>>> of
>>>>>>>>> clean, sensible commits, then I would favor merging with a
>>>> merge
>>>>>>>>> commit that shows the PR number.
>>>>>>>>>
>>>>>>>>> My issue with "rebase and merge" is that the commit history
>>>> would
>>>>> not
>>>>>>>>> show the PR number.
>>>>>>>>>
>>>>>>>>> I think that having the commits show the PR number would make
>>>> it
>>>> a
>>>>>>>>> little easier for whoever makes the release to make the release
>>>>> notes
>>>>>>>>> with the PR numbers.
>>>>>>>>>
>>>>>>>>> Are there any recent examples of "a lot of commits from the
>>>> same
>>>>> PR
>>>>>>>>> with meaningless commit messages when changes were requested"?
>>>>>>>>>
>>>>>>>>> Maybe off-topic: I wonder if we should look for multiple
>>>> committers
>>>>>> to
>>>>>>>>> approve an external contribution before merging?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, Oct 1, 2019 at 7:18 AM julio cesar sanchez
>>>>>>>>> <jc...@gmail.com> wrote:
>>>>>>>>>> Last year, Jan started a thread with different topics and one
>>>> of
>>>>>> them
>>>>>>>> was
>>>>>>>>>> to have a merge convention. I copy the text:
>>>>>>>>>>
>>>>>>>>>>> 3. Merge Conventions / Protected Branch:
>>>>>>>>>>> Connected to all that is my suggestion to protect the
>>>> `master`
>>>>>> branch
>>>>>>>> so
>>>>>>>>>> that by default nobody can commit there - all changes have to
>>>> be
>>>>>> made
>>>>>>>> via
>>>>>>>>>> Pull Requests. Pull Requests are by default merged via the
>>>>> "Squash +
>>>>>>>> Merge"
>>>>>>>>>> button / functionality so that all commits are squashed into
>>>> one
>>>>>> clean
>>>>>>>>>> commit per change. This also enforces the commit message
>>>>> structure I
>>>>>>>> posted
>>>>>>>>>> above. (Of course committers can choose to _not_ use Squash +
>>>>> Merge
>>>>>> if
>>>>>>>>>> appropriate for the PR - e.g. when cherry picking commits
>>>> from a
>>>>>>> release
>>>>>>>>>> branch or similar).
>>>>>>>>>>
>>>>>>>>>>> What do you think about this suggestion?
>>>>>>>>>> Looks like we didn't agree on anything, but can we agree now?
>>>>>>>>>>
>>>>>>>>>> I've checked a few repos and some of them have a lot of
>>>> commits
>>>>> from
>>>>>>> the
>>>>>>>>>> same PR with meaningless commit messages when changes were
>>>>>> requested,
>>>>>>>> plus
>>>>>>>>>> the ugly "merge PR ### from YYY" that makes the commit history
>>>>> hard
>>>>>> to
>>>>>>>>>> follow and hard to cherry pick if needed.
>>>>>>>>>>
>>>>>>>>>> Since I'm not sure if we can protect branches, I'll focus only
>>>> on
>>>>>> the
>>>>>>>> merge
>>>>>>>>>> convention.
>>>>>>>>>>
>>>>>>>>>> Can we all agree on using the "squash + merge" for user PRs,
>>>>> unless
>>>>>> we
>>>>>>>>>> think the different commits makes sense, in this case we
>>>> should
>>>>> try
>>>>>>> the
>>>>>>>>>> "rebase and merge" button.
>>>>>>>>>>
>>>>>>>>>> I vote +1
>>>>>>>>>
>>>>> ---------------------------------------------------------------------
>>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
>>>>>>>>> For additional commands, e-mail: dev-help@cordova.apache.org
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>> ---------------------------------------------------------------------
>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
>>>>>>>> For additional commands, e-mail: dev-help@cordova.apache.org
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>> --
>>>> Regards,
>>>> Gandhi
>>>>
>>>> "The best way to find urself is to lose urself in the service of others
>>>> !!!"
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
>> For additional commands, e-mail: dev-help@cordova.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> For additional commands, e-mail: dev-help@cordova.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
Re: vote: PR merge convention
Posted by Tim Brust <ti...@gmail.com>.
+1 to Niklas suggestion :)
Sent from my iPhone
> On 2. May 2020, at 6:54 PM, Niklas Merz <ni...@apache.org> wrote:
>
> We could try to enforce this setting with .asf.yml. I saw this posted on another list.
>
> See: https://cwiki.apache.org/confluence/display/INFRA/.asf.yaml+features+for+git+repositories#id-.asf.yamlfeaturesforgitrepositories-Mergebuttons
>
> Should we roll this out to all repos?
>
> May 2, 2020 1:38 PM, "julio cesar sanchez" <jc...@gmail.com> wrote:
>
>> Just a reminder that we agreed on using the squash and merge, but I still
>> see regular merge commits in a few repos from time to time.
>>
>>> El El sáb, 5 oct 2019 a las 19:32, gandhi rajan <ga...@gmail.com>
>>> escribió:
>>>
>>> + 1 for squash and merge as it makes the repo history cleaner.
>>>
>>>> On Sat, Oct 5, 2019 at 8:34 PM <ra...@gmail.com> wrote:
>>>
>>> +1 for "Squash and merge" as the default strategy
>>>
>>> Regarding "Create a merge commit":
>>> At times, I find this option valuable too. Consider a PR that cleans up a
>>> test suite. As part of that cleanup I might re-order the test cases. As
>>> re-ordering produces a noisy diff, I usually isolate it in its own
>>> commit.
>>> I would not want to merge this commit with the others as that would lead
>>> to
>>> a commit with a very incomprehensible diff. So in this case I would favor
>>> leaving the commits separate and doing an actual merge to group them
>>> together in the global history.
>>>
>>> Am Fr., 4. Okt. 2019 um 17:03 Uhr schrieb julio cesar sanchez <
>>> jcesarmobile@gmail.com>:
>>>
>>>> Sorry if it wasn't clear, I said I was leaving the protecting master
>>> branch
>>>> out of the vote.
>>>>
>>>> Looks like we all agree on using "Squash and merge" as default, unless
>>> it
>>>> makes more sense to use one of the other options.
>>>>
>>>> El jue., 3 oct. 2019 a las 3:43, Bryan Ellis (<er...@apache.org>)
>>>> escribió:
>>>>
>>>>> -1 for protected master branches.
>>>>> Protecting a branch is a great idea except it does not work with our
>>>>> current workflow process. COHO commits directly onto the master
>>> branch
>>>> for
>>>>> releases. We would have to spend more time planning and changing our
>>>> entire
>>>>> current workflow process to eliminate direct commits if we wanted to
>>>>> protect branches. There is alternative such keeping master open for
>>>> direct
>>>>> commits and development while creating a protected "production"
>>> branch.
>>>>> Anyways this part of the discussion is off-topic and could be another
>>>>> discussion... Anyways, stated above I am downvoting protected
>>> branches
>>>> for
>>>>> now.
>>>>>
>>>>> +1 for "Squash and merge"
>>>>> Makes a nice single commit with the PR number and without the extra
>>> merge
>>>>> commit.
>>>>>
>>>>> +1 for "Rebase and merge"
>>>>> There are use cases where this can work perfectly.
>>>>> For example, Cordova-Electron has a `draft-2.0.0` branch which is
>>>> targeting
>>>>> the next major release. Major PRs were merged into that branch with
>>>> "Squash
>>>>> and merge". This means all PRs have nice PR# information in the
>>> title.
>>>>> A PR will later be created to merge this branch onto master. "Rebase
>>> and
>>>>> merge" will be used and will not create the merge commit message and
>>> will
>>>>> not squash.
>>>>>
>>>>> -1 for "Create a merge commit"
>>>>> Not in favor of the extra merge commit. I think in most cases one PR
>>>> should
>>>>> focus on one task so that means it could be squashed when there are
>>>>> multiple commits. If "Create a merge commit" is used, each commit
>>> will
>>> be
>>>>> merged and does not contain the PR# in the title. When creating
>>> release
>>>>> notes, I need to manually review those commits to identify what PR it
>>>> came
>>>>> from to include the PR linking. Some times, depending on if they are
>>> all
>>>>> related commits, I need to manually group them and create a
>>> meaningful
>>>>> title for the release notes which I would prefer to have been done
>>>>> beforehand.
>>>>>
>>>>>
>>>>> On Wed, Oct 2, 2019 at 12:51 AM Jesse <pu...@gmail.com>
>>> wrote:
>>>>>
>>>>>> -1 for protected master branches, we are a small group of
>>> committers
>>>> and
>>>>>> don't need rules to keep us honest.
>>>>>> Protecting master would involve infra, as we cannot manage the
>>> minutia
>>>> in
>>>>>> github. I think we all do this anyway except for rare occasions.
>>>>>>
>>>>>> +1 for squash and merge, as long as the meaningful individual
>>> commit
>>>>>> messages get consolidated into the 1 commit.
>>>>>>
>>>>>> On Tue, Oct 1, 2019 at 8:36 AM Norman Breau <
>>> norman@normanbreau.com>
>>>>>> wrote:
>>>>>>
>>>>>>> +1 to protect the master branch.
>>>>>>>
>>>>>>> Forcing PR will help organize commits if we need to go back in
>>>>>>> time to determine the reason why a change was made as the
>>>>>>> commit in github will show the corresponding PR. Which will
>>>>>>> (hopefully) be properly filled out with context and motivation,
>>>>>>> as well as the issues that the PR resolves.
>>>>>>>
>>>>>>> +1 for "squash + merge" as a default strategy.
>>>>>>>
>>>>>>> I feel like most of the time having all the individual commits
>>> that
>>>>>>> makes up a PR is not really necessary. Most of the time it's
>>>>>>> bloated with tweaks fixing errors that was introduced during the
>>>>>>> development of the PR or perhaps refactoring for code style, etc.
>>>>>>> If there are instances where squash shouldn't be used, that can
>>>>>>> be decided on a per-case basis in my opinion.
>>>>>>>
>>>>>>> On 2019-10-01 10:37 a.m., Chris Brody wrote:
>>>>>>>> I would agree that "squash + merge" should be used in *most*
>>> cases,
>>>>>>>> and I think there is no dispute on this point.
>>>>>>>>
>>>>>>>> In the few cases where there are too many things for a "squash
>>> +
>>>>>>>> merge" commit, and we have the changes down to a limited number
>>> of
>>>>>>>> clean, sensible commits, then I would favor merging with a
>>> merge
>>>>>>>> commit that shows the PR number.
>>>>>>>>
>>>>>>>> My issue with "rebase and merge" is that the commit history
>>> would
>>>> not
>>>>>>>> show the PR number.
>>>>>>>>
>>>>>>>> I think that having the commits show the PR number would make
>>> it
>>> a
>>>>>>>> little easier for whoever makes the release to make the release
>>>> notes
>>>>>>>> with the PR numbers.
>>>>>>>>
>>>>>>>> Are there any recent examples of "a lot of commits from the
>>> same
>>>> PR
>>>>>>>> with meaningless commit messages when changes were requested"?
>>>>>>>>
>>>>>>>> Maybe off-topic: I wonder if we should look for multiple
>>> committers
>>>>> to
>>>>>>>> approve an external contribution before merging?
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Oct 1, 2019 at 7:18 AM julio cesar sanchez
>>>>>>>> <jc...@gmail.com> wrote:
>>>>>>>>> Last year, Jan started a thread with different topics and one
>>> of
>>>>> them
>>>>>>> was
>>>>>>>>> to have a merge convention. I copy the text:
>>>>>>>>>
>>>>>>>>>> 3. Merge Conventions / Protected Branch:
>>>>>>>>>> Connected to all that is my suggestion to protect the
>>> `master`
>>>>> branch
>>>>>>> so
>>>>>>>>> that by default nobody can commit there - all changes have to
>>> be
>>>>> made
>>>>>>> via
>>>>>>>>> Pull Requests. Pull Requests are by default merged via the
>>>> "Squash +
>>>>>>> Merge"
>>>>>>>>> button / functionality so that all commits are squashed into
>>> one
>>>>> clean
>>>>>>>>> commit per change. This also enforces the commit message
>>>> structure I
>>>>>>> posted
>>>>>>>>> above. (Of course committers can choose to _not_ use Squash +
>>>> Merge
>>>>> if
>>>>>>>>> appropriate for the PR - e.g. when cherry picking commits
>>> from a
>>>>>> release
>>>>>>>>> branch or similar).
>>>>>>>>>
>>>>>>>>>> What do you think about this suggestion?
>>>>>>>>> Looks like we didn't agree on anything, but can we agree now?
>>>>>>>>>
>>>>>>>>> I've checked a few repos and some of them have a lot of
>>> commits
>>>> from
>>>>>> the
>>>>>>>>> same PR with meaningless commit messages when changes were
>>>>> requested,
>>>>>>> plus
>>>>>>>>> the ugly "merge PR ### from YYY" that makes the commit history
>>>> hard
>>>>> to
>>>>>>>>> follow and hard to cherry pick if needed.
>>>>>>>>>
>>>>>>>>> Since I'm not sure if we can protect branches, I'll focus only
>>> on
>>>>> the
>>>>>>> merge
>>>>>>>>> convention.
>>>>>>>>>
>>>>>>>>> Can we all agree on using the "squash + merge" for user PRs,
>>>> unless
>>>>> we
>>>>>>>>> think the different commits makes sense, in this case we
>>> should
>>>> try
>>>>>> the
>>>>>>>>> "rebase and merge" button.
>>>>>>>>>
>>>>>>>>> I vote +1
>>>>>>>>
>>>> ---------------------------------------------------------------------
>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
>>>>>>>> For additional commands, e-mail: dev-help@cordova.apache.org
>>>>>>>>
>>>>>>>
>>>>>>>
>>> ---------------------------------------------------------------------
>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
>>>>>>> For additional commands, e-mail: dev-help@cordova.apache.org
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>> --
>>> Regards,
>>> Gandhi
>>>
>>> "The best way to find urself is to lose urself in the service of others
>>> !!!"
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> For additional commands, e-mail: dev-help@cordova.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
Re: vote: PR merge convention
Posted by Niklas Merz <ni...@apache.org>.
We could try to enforce this setting with .asf.yml. I saw this posted on another list.
See: https://cwiki.apache.org/confluence/display/INFRA/.asf.yaml+features+for+git+repositories#id-.asf.yamlfeaturesforgitrepositories-Mergebuttons
Should we roll this out to all repos?
May 2, 2020 1:38 PM, "julio cesar sanchez" <jc...@gmail.com> wrote:
> Just a reminder that we agreed on using the squash and merge, but I still
> see regular merge commits in a few repos from time to time.
>
> El El sáb, 5 oct 2019 a las 19:32, gandhi rajan <ga...@gmail.com>
> escribió:
>
>> + 1 for squash and merge as it makes the repo history cleaner.
>>
>> On Sat, Oct 5, 2019 at 8:34 PM <ra...@gmail.com> wrote:
>>
>> +1 for "Squash and merge" as the default strategy
>>
>> Regarding "Create a merge commit":
>> At times, I find this option valuable too. Consider a PR that cleans up a
>> test suite. As part of that cleanup I might re-order the test cases. As
>> re-ordering produces a noisy diff, I usually isolate it in its own
>> commit.
>> I would not want to merge this commit with the others as that would lead
>> to
>> a commit with a very incomprehensible diff. So in this case I would favor
>> leaving the commits separate and doing an actual merge to group them
>> together in the global history.
>>
>> Am Fr., 4. Okt. 2019 um 17:03 Uhr schrieb julio cesar sanchez <
>> jcesarmobile@gmail.com>:
>>
>>> Sorry if it wasn't clear, I said I was leaving the protecting master
>> branch
>>> out of the vote.
>>>
>>> Looks like we all agree on using "Squash and merge" as default, unless
>> it
>>> makes more sense to use one of the other options.
>>>
>>> El jue., 3 oct. 2019 a las 3:43, Bryan Ellis (<er...@apache.org>)
>>> escribió:
>>>
>>>> -1 for protected master branches.
>>>> Protecting a branch is a great idea except it does not work with our
>>>> current workflow process. COHO commits directly onto the master
>> branch
>>> for
>>>> releases. We would have to spend more time planning and changing our
>>> entire
>>>> current workflow process to eliminate direct commits if we wanted to
>>>> protect branches. There is alternative such keeping master open for
>>> direct
>>>> commits and development while creating a protected "production"
>> branch.
>>>> Anyways this part of the discussion is off-topic and could be another
>>>> discussion... Anyways, stated above I am downvoting protected
>> branches
>>> for
>>>> now.
>>>>
>>>> +1 for "Squash and merge"
>>>> Makes a nice single commit with the PR number and without the extra
>> merge
>>>> commit.
>>>>
>>>> +1 for "Rebase and merge"
>>>> There are use cases where this can work perfectly.
>>>> For example, Cordova-Electron has a `draft-2.0.0` branch which is
>>> targeting
>>>> the next major release. Major PRs were merged into that branch with
>>> "Squash
>>>> and merge". This means all PRs have nice PR# information in the
>> title.
>>>> A PR will later be created to merge this branch onto master. "Rebase
>> and
>>>> merge" will be used and will not create the merge commit message and
>> will
>>>> not squash.
>>>>
>>>> -1 for "Create a merge commit"
>>>> Not in favor of the extra merge commit. I think in most cases one PR
>>> should
>>>> focus on one task so that means it could be squashed when there are
>>>> multiple commits. If "Create a merge commit" is used, each commit
>> will
>> be
>>>> merged and does not contain the PR# in the title. When creating
>> release
>>>> notes, I need to manually review those commits to identify what PR it
>>> came
>>>> from to include the PR linking. Some times, depending on if they are
>> all
>>>> related commits, I need to manually group them and create a
>> meaningful
>>>> title for the release notes which I would prefer to have been done
>>>> beforehand.
>>>>
>>>>
>>>> On Wed, Oct 2, 2019 at 12:51 AM Jesse <pu...@gmail.com>
>> wrote:
>>>>
>>>>> -1 for protected master branches, we are a small group of
>> committers
>>> and
>>>>> don't need rules to keep us honest.
>>>>> Protecting master would involve infra, as we cannot manage the
>> minutia
>>> in
>>>>> github. I think we all do this anyway except for rare occasions.
>>>>>
>>>>> +1 for squash and merge, as long as the meaningful individual
>> commit
>>>>> messages get consolidated into the 1 commit.
>>>>>
>>>>> On Tue, Oct 1, 2019 at 8:36 AM Norman Breau <
>> norman@normanbreau.com>
>>>>> wrote:
>>>>>
>>>>>> +1 to protect the master branch.
>>>>>>
>>>>>> Forcing PR will help organize commits if we need to go back in
>>>>>> time to determine the reason why a change was made as the
>>>>>> commit in github will show the corresponding PR. Which will
>>>>>> (hopefully) be properly filled out with context and motivation,
>>>>>> as well as the issues that the PR resolves.
>>>>>>
>>>>>> +1 for "squash + merge" as a default strategy.
>>>>>>
>>>>>> I feel like most of the time having all the individual commits
>> that
>>>>>> makes up a PR is not really necessary. Most of the time it's
>>>>>> bloated with tweaks fixing errors that was introduced during the
>>>>>> development of the PR or perhaps refactoring for code style, etc.
>>>>>> If there are instances where squash shouldn't be used, that can
>>>>>> be decided on a per-case basis in my opinion.
>>>>>>
>>>>>> On 2019-10-01 10:37 a.m., Chris Brody wrote:
>>>>>>> I would agree that "squash + merge" should be used in *most*
>> cases,
>>>>>>> and I think there is no dispute on this point.
>>>>>>>
>>>>>>> In the few cases where there are too many things for a "squash
>> +
>>>>>>> merge" commit, and we have the changes down to a limited number
>> of
>>>>>>> clean, sensible commits, then I would favor merging with a
>> merge
>>>>>>> commit that shows the PR number.
>>>>>>>
>>>>>>> My issue with "rebase and merge" is that the commit history
>> would
>>> not
>>>>>>> show the PR number.
>>>>>>>
>>>>>>> I think that having the commits show the PR number would make
>> it
>> a
>>>>>>> little easier for whoever makes the release to make the release
>>> notes
>>>>>>> with the PR numbers.
>>>>>>>
>>>>>>> Are there any recent examples of "a lot of commits from the
>> same
>>> PR
>>>>>>> with meaningless commit messages when changes were requested"?
>>>>>>>
>>>>>>> Maybe off-topic: I wonder if we should look for multiple
>> committers
>>>> to
>>>>>>> approve an external contribution before merging?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Tue, Oct 1, 2019 at 7:18 AM julio cesar sanchez
>>>>>>> <jc...@gmail.com> wrote:
>>>>>>>> Last year, Jan started a thread with different topics and one
>> of
>>>> them
>>>>>> was
>>>>>>>> to have a merge convention. I copy the text:
>>>>>>>>
>>>>>>>>> 3. Merge Conventions / Protected Branch:
>>>>>>>>> Connected to all that is my suggestion to protect the
>> `master`
>>>> branch
>>>>>> so
>>>>>>>> that by default nobody can commit there - all changes have to
>> be
>>>> made
>>>>>> via
>>>>>>>> Pull Requests. Pull Requests are by default merged via the
>>> "Squash +
>>>>>> Merge"
>>>>>>>> button / functionality so that all commits are squashed into
>> one
>>>> clean
>>>>>>>> commit per change. This also enforces the commit message
>>> structure I
>>>>>> posted
>>>>>>>> above. (Of course committers can choose to _not_ use Squash +
>>> Merge
>>>> if
>>>>>>>> appropriate for the PR - e.g. when cherry picking commits
>> from a
>>>>> release
>>>>>>>> branch or similar).
>>>>>>>>
>>>>>>>>> What do you think about this suggestion?
>>>>>>>> Looks like we didn't agree on anything, but can we agree now?
>>>>>>>>
>>>>>>>> I've checked a few repos and some of them have a lot of
>> commits
>>> from
>>>>> the
>>>>>>>> same PR with meaningless commit messages when changes were
>>>> requested,
>>>>>> plus
>>>>>>>> the ugly "merge PR ### from YYY" that makes the commit history
>>> hard
>>>> to
>>>>>>>> follow and hard to cherry pick if needed.
>>>>>>>>
>>>>>>>> Since I'm not sure if we can protect branches, I'll focus only
>> on
>>>> the
>>>>>> merge
>>>>>>>> convention.
>>>>>>>>
>>>>>>>> Can we all agree on using the "squash + merge" for user PRs,
>>> unless
>>>> we
>>>>>>>> think the different commits makes sense, in this case we
>> should
>>> try
>>>>> the
>>>>>>>> "rebase and merge" button.
>>>>>>>>
>>>>>>>> I vote +1
>>>>>>>
>>> ---------------------------------------------------------------------
>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
>>>>>>> For additional commands, e-mail: dev-help@cordova.apache.org
>>>>>>>
>>>>>>
>>>>>>
>> ---------------------------------------------------------------------
>>>>>> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
>>>>>> For additional commands, e-mail: dev-help@cordova.apache.org
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>> --
>> Regards,
>> Gandhi
>>
>> "The best way to find urself is to lose urself in the service of others
>> !!!"
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
Re: vote: PR merge convention
Posted by julio cesar sanchez <jc...@gmail.com>.
Just a reminder that we agreed on using the squash and merge, but I still
see regular merge commits in a few repos from time to time.
El El sáb, 5 oct 2019 a las 19:32, gandhi rajan <ga...@gmail.com>
escribió:
> + 1 for squash and merge as it makes the repo history cleaner.
>
> On Sat, Oct 5, 2019 at 8:34 PM <ra...@gmail.com> wrote:
>
> > +1 for "Squash and merge" as the default strategy
> >
> > Regarding "Create a merge commit":
> > At times, I find this option valuable too. Consider a PR that cleans up a
> > test suite. As part of that cleanup I might re-order the test cases. As
> > re-ordering produces a noisy diff, I usually isolate it in its own
> commit.
> > I would not want to merge this commit with the others as that would lead
> to
> > a commit with a very incomprehensible diff. So in this case I would favor
> > leaving the commits separate and doing an actual merge to group them
> > together in the global history.
> >
> > Am Fr., 4. Okt. 2019 um 17:03 Uhr schrieb julio cesar sanchez <
> > jcesarmobile@gmail.com>:
> >
> > > Sorry if it wasn't clear, I said I was leaving the protecting master
> > branch
> > > out of the vote.
> > >
> > > Looks like we all agree on using "Squash and merge" as default, unless
> it
> > > makes more sense to use one of the other options.
> > >
> > > El jue., 3 oct. 2019 a las 3:43, Bryan Ellis (<er...@apache.org>)
> > > escribió:
> > >
> > > > -1 for protected master branches.
> > > > Protecting a branch is a great idea except it does not work with our
> > > > current workflow process. COHO commits directly onto the master
> branch
> > > for
> > > > releases. We would have to spend more time planning and changing our
> > > entire
> > > > current workflow process to eliminate direct commits if we wanted to
> > > > protect branches. There is alternative such keeping master open for
> > > direct
> > > > commits and development while creating a protected "production"
> branch.
> > > > Anyways this part of the discussion is off-topic and could be another
> > > > discussion... Anyways, stated above I am downvoting protected
> branches
> > > for
> > > > now.
> > > >
> > > > +1 for "Squash and merge"
> > > > Makes a nice single commit with the PR number and without the extra
> > merge
> > > > commit.
> > > >
> > > > +1 for "Rebase and merge"
> > > > There are use cases where this can work perfectly.
> > > > For example, Cordova-Electron has a `draft-2.0.0` branch which is
> > > targeting
> > > > the next major release. Major PRs were merged into that branch with
> > > "Squash
> > > > and merge". This means all PRs have nice PR# information in the
> title.
> > > > A PR will later be created to merge this branch onto master. "Rebase
> > and
> > > > merge" will be used and will not create the merge commit message and
> > will
> > > > not squash.
> > > >
> > > > -1 for "Create a merge commit"
> > > > Not in favor of the extra merge commit. I think in most cases one PR
> > > should
> > > > focus on one task so that means it could be squashed when there are
> > > > multiple commits. If "Create a merge commit" is used, each commit
> will
> > be
> > > > merged and does not contain the PR# in the title. When creating
> release
> > > > notes, I need to manually review those commits to identify what PR it
> > > came
> > > > from to include the PR linking. Some times, depending on if they are
> > all
> > > > related commits, I need to manually group them and create a
> meaningful
> > > > title for the release notes which I would prefer to have been done
> > > > beforehand.
> > > >
> > > >
> > > > On Wed, Oct 2, 2019 at 12:51 AM Jesse <pu...@gmail.com>
> wrote:
> > > >
> > > > > -1 for protected master branches, we are a small group of
> committers
> > > and
> > > > > don't need rules to keep us honest.
> > > > > Protecting master would involve infra, as we cannot manage the
> > minutia
> > > in
> > > > > github. I think we all do this anyway except for rare occasions.
> > > > >
> > > > > +1 for squash and merge, as long as the meaningful individual
> commit
> > > > > messages get consolidated into the 1 commit.
> > > > >
> > > > > On Tue, Oct 1, 2019 at 8:36 AM Norman Breau <
> norman@normanbreau.com>
> > > > > wrote:
> > > > >
> > > > > > +1 to protect the master branch.
> > > > > >
> > > > > > Forcing PR will help organize commits if we need to go back in
> > > > > > time to determine the reason why a change was made as the
> > > > > > commit in github will show the corresponding PR. Which will
> > > > > > (hopefully) be properly filled out with context and motivation,
> > > > > > as well as the issues that the PR resolves.
> > > > > >
> > > > > > +1 for "squash + merge" as a default strategy.
> > > > > >
> > > > > > I feel like most of the time having all the individual commits
> that
> > > > > > makes up a PR is not really necessary. Most of the time it's
> > > > > > bloated with tweaks fixing errors that was introduced during the
> > > > > > development of the PR or perhaps refactoring for code style, etc.
> > > > > > If there are instances where squash shouldn't be used, that can
> > > > > > be decided on a per-case basis in my opinion.
> > > > > >
> > > > > > On 2019-10-01 10:37 a.m., Chris Brody wrote:
> > > > > > > I would agree that "squash + merge" should be used in *most*
> > cases,
> > > > > > > and I think there is no dispute on this point.
> > > > > > >
> > > > > > > In the few cases where there are too many things for a "squash
> +
> > > > > > > merge" commit, and we have the changes down to a limited number
> > of
> > > > > > > clean, sensible commits, then I would favor merging with a
> merge
> > > > > > > commit that shows the PR number.
> > > > > > >
> > > > > > > My issue with "rebase and merge" is that the commit history
> would
> > > not
> > > > > > > show the PR number.
> > > > > > >
> > > > > > > I think that having the commits show the PR number would make
> it
> > a
> > > > > > > little easier for whoever makes the release to make the release
> > > notes
> > > > > > > with the PR numbers.
> > > > > > >
> > > > > > > Are there any recent examples of "a lot of commits from the
> same
> > > PR
> > > > > > > with meaningless commit messages when changes were requested"?
> > > > > > >
> > > > > > > Maybe off-topic: I wonder if we should look for multiple
> > committers
> > > > to
> > > > > > > approve an external contribution before merging?
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On Tue, Oct 1, 2019 at 7:18 AM julio cesar sanchez
> > > > > > > <jc...@gmail.com> wrote:
> > > > > > >> Last year, Jan started a thread with different topics and one
> of
> > > > them
> > > > > > was
> > > > > > >> to have a merge convention. I copy the text:
> > > > > > >>
> > > > > > >>> 3. Merge Conventions / Protected Branch:
> > > > > > >>> Connected to all that is my suggestion to protect the
> `master`
> > > > branch
> > > > > > so
> > > > > > >> that by default nobody can commit there - all changes have to
> be
> > > > made
> > > > > > via
> > > > > > >> Pull Requests. Pull Requests are by default merged via the
> > > "Squash +
> > > > > > Merge"
> > > > > > >> button / functionality so that all commits are squashed into
> one
> > > > clean
> > > > > > >> commit per change. This also enforces the commit message
> > > structure I
> > > > > > posted
> > > > > > >> above. (Of course committers can choose to _not_ use Squash +
> > > Merge
> > > > if
> > > > > > >> appropriate for the PR - e.g. when cherry picking commits
> from a
> > > > > release
> > > > > > >> branch or similar).
> > > > > > >>
> > > > > > >>> What do you think about this suggestion?
> > > > > > >> Looks like we didn't agree on anything, but can we agree now?
> > > > > > >>
> > > > > > >> I've checked a few repos and some of them have a lot of
> commits
> > > from
> > > > > the
> > > > > > >> same PR with meaningless commit messages when changes were
> > > > requested,
> > > > > > plus
> > > > > > >> the ugly "merge PR ### from YYY" that makes the commit history
> > > hard
> > > > to
> > > > > > >> follow and hard to cherry pick if needed.
> > > > > > >>
> > > > > > >> Since I'm not sure if we can protect branches, I'll focus only
> > on
> > > > the
> > > > > > merge
> > > > > > >> convention.
> > > > > > >>
> > > > > > >> Can we all agree on using the "squash + merge" for user PRs,
> > > unless
> > > > we
> > > > > > >> think the different commits makes sense, in this case we
> should
> > > try
> > > > > the
> > > > > > >> "rebase and merge" button.
> > > > > > >>
> > > > > > >> I vote +1
> > > > > > >
> > > ---------------------------------------------------------------------
> > > > > > > To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > > > > > > For additional commands, e-mail: dev-help@cordova.apache.org
> > > > > > >
> > > > > >
> > > > > >
> > ---------------------------------------------------------------------
> > > > > > To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > > > > > For additional commands, e-mail: dev-help@cordova.apache.org
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
>
> --
> Regards,
> Gandhi
>
> "The best way to find urself is to lose urself in the service of others
> !!!"
>
Re: vote: PR merge convention
Posted by gandhi rajan <ga...@gmail.com>.
+ 1 for squash and merge as it makes the repo history cleaner.
On Sat, Oct 5, 2019 at 8:34 PM <ra...@gmail.com> wrote:
> +1 for "Squash and merge" as the default strategy
>
> Regarding "Create a merge commit":
> At times, I find this option valuable too. Consider a PR that cleans up a
> test suite. As part of that cleanup I might re-order the test cases. As
> re-ordering produces a noisy diff, I usually isolate it in its own commit.
> I would not want to merge this commit with the others as that would lead to
> a commit with a very incomprehensible diff. So in this case I would favor
> leaving the commits separate and doing an actual merge to group them
> together in the global history.
>
> Am Fr., 4. Okt. 2019 um 17:03 Uhr schrieb julio cesar sanchez <
> jcesarmobile@gmail.com>:
>
> > Sorry if it wasn't clear, I said I was leaving the protecting master
> branch
> > out of the vote.
> >
> > Looks like we all agree on using "Squash and merge" as default, unless it
> > makes more sense to use one of the other options.
> >
> > El jue., 3 oct. 2019 a las 3:43, Bryan Ellis (<er...@apache.org>)
> > escribió:
> >
> > > -1 for protected master branches.
> > > Protecting a branch is a great idea except it does not work with our
> > > current workflow process. COHO commits directly onto the master branch
> > for
> > > releases. We would have to spend more time planning and changing our
> > entire
> > > current workflow process to eliminate direct commits if we wanted to
> > > protect branches. There is alternative such keeping master open for
> > direct
> > > commits and development while creating a protected "production" branch.
> > > Anyways this part of the discussion is off-topic and could be another
> > > discussion... Anyways, stated above I am downvoting protected branches
> > for
> > > now.
> > >
> > > +1 for "Squash and merge"
> > > Makes a nice single commit with the PR number and without the extra
> merge
> > > commit.
> > >
> > > +1 for "Rebase and merge"
> > > There are use cases where this can work perfectly.
> > > For example, Cordova-Electron has a `draft-2.0.0` branch which is
> > targeting
> > > the next major release. Major PRs were merged into that branch with
> > "Squash
> > > and merge". This means all PRs have nice PR# information in the title.
> > > A PR will later be created to merge this branch onto master. "Rebase
> and
> > > merge" will be used and will not create the merge commit message and
> will
> > > not squash.
> > >
> > > -1 for "Create a merge commit"
> > > Not in favor of the extra merge commit. I think in most cases one PR
> > should
> > > focus on one task so that means it could be squashed when there are
> > > multiple commits. If "Create a merge commit" is used, each commit will
> be
> > > merged and does not contain the PR# in the title. When creating release
> > > notes, I need to manually review those commits to identify what PR it
> > came
> > > from to include the PR linking. Some times, depending on if they are
> all
> > > related commits, I need to manually group them and create a meaningful
> > > title for the release notes which I would prefer to have been done
> > > beforehand.
> > >
> > >
> > > On Wed, Oct 2, 2019 at 12:51 AM Jesse <pu...@gmail.com> wrote:
> > >
> > > > -1 for protected master branches, we are a small group of committers
> > and
> > > > don't need rules to keep us honest.
> > > > Protecting master would involve infra, as we cannot manage the
> minutia
> > in
> > > > github. I think we all do this anyway except for rare occasions.
> > > >
> > > > +1 for squash and merge, as long as the meaningful individual commit
> > > > messages get consolidated into the 1 commit.
> > > >
> > > > On Tue, Oct 1, 2019 at 8:36 AM Norman Breau <no...@normanbreau.com>
> > > > wrote:
> > > >
> > > > > +1 to protect the master branch.
> > > > >
> > > > > Forcing PR will help organize commits if we need to go back in
> > > > > time to determine the reason why a change was made as the
> > > > > commit in github will show the corresponding PR. Which will
> > > > > (hopefully) be properly filled out with context and motivation,
> > > > > as well as the issues that the PR resolves.
> > > > >
> > > > > +1 for "squash + merge" as a default strategy.
> > > > >
> > > > > I feel like most of the time having all the individual commits that
> > > > > makes up a PR is not really necessary. Most of the time it's
> > > > > bloated with tweaks fixing errors that was introduced during the
> > > > > development of the PR or perhaps refactoring for code style, etc.
> > > > > If there are instances where squash shouldn't be used, that can
> > > > > be decided on a per-case basis in my opinion.
> > > > >
> > > > > On 2019-10-01 10:37 a.m., Chris Brody wrote:
> > > > > > I would agree that "squash + merge" should be used in *most*
> cases,
> > > > > > and I think there is no dispute on this point.
> > > > > >
> > > > > > In the few cases where there are too many things for a "squash +
> > > > > > merge" commit, and we have the changes down to a limited number
> of
> > > > > > clean, sensible commits, then I would favor merging with a merge
> > > > > > commit that shows the PR number.
> > > > > >
> > > > > > My issue with "rebase and merge" is that the commit history would
> > not
> > > > > > show the PR number.
> > > > > >
> > > > > > I think that having the commits show the PR number would make it
> a
> > > > > > little easier for whoever makes the release to make the release
> > notes
> > > > > > with the PR numbers.
> > > > > >
> > > > > > Are there any recent examples of "a lot of commits from the same
> > PR
> > > > > > with meaningless commit messages when changes were requested"?
> > > > > >
> > > > > > Maybe off-topic: I wonder if we should look for multiple
> committers
> > > to
> > > > > > approve an external contribution before merging?
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Tue, Oct 1, 2019 at 7:18 AM julio cesar sanchez
> > > > > > <jc...@gmail.com> wrote:
> > > > > >> Last year, Jan started a thread with different topics and one of
> > > them
> > > > > was
> > > > > >> to have a merge convention. I copy the text:
> > > > > >>
> > > > > >>> 3. Merge Conventions / Protected Branch:
> > > > > >>> Connected to all that is my suggestion to protect the `master`
> > > branch
> > > > > so
> > > > > >> that by default nobody can commit there - all changes have to be
> > > made
> > > > > via
> > > > > >> Pull Requests. Pull Requests are by default merged via the
> > "Squash +
> > > > > Merge"
> > > > > >> button / functionality so that all commits are squashed into one
> > > clean
> > > > > >> commit per change. This also enforces the commit message
> > structure I
> > > > > posted
> > > > > >> above. (Of course committers can choose to _not_ use Squash +
> > Merge
> > > if
> > > > > >> appropriate for the PR - e.g. when cherry picking commits from a
> > > > release
> > > > > >> branch or similar).
> > > > > >>
> > > > > >>> What do you think about this suggestion?
> > > > > >> Looks like we didn't agree on anything, but can we agree now?
> > > > > >>
> > > > > >> I've checked a few repos and some of them have a lot of commits
> > from
> > > > the
> > > > > >> same PR with meaningless commit messages when changes were
> > > requested,
> > > > > plus
> > > > > >> the ugly "merge PR ### from YYY" that makes the commit history
> > hard
> > > to
> > > > > >> follow and hard to cherry pick if needed.
> > > > > >>
> > > > > >> Since I'm not sure if we can protect branches, I'll focus only
> on
> > > the
> > > > > merge
> > > > > >> convention.
> > > > > >>
> > > > > >> Can we all agree on using the "squash + merge" for user PRs,
> > unless
> > > we
> > > > > >> think the different commits makes sense, in this case we should
> > try
> > > > the
> > > > > >> "rebase and merge" button.
> > > > > >>
> > > > > >> I vote +1
> > > > > >
> > ---------------------------------------------------------------------
> > > > > > To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > > > > > For additional commands, e-mail: dev-help@cordova.apache.org
> > > > > >
> > > > >
> > > > >
> ---------------------------------------------------------------------
> > > > > To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > > > > For additional commands, e-mail: dev-help@cordova.apache.org
> > > > >
> > > > >
> > > >
> > >
> >
>
--
Regards,
Gandhi
"The best way to find urself is to lose urself in the service of others !!!"
Re: vote: PR merge convention
Posted by ra...@gmail.com.
+1 for "Squash and merge" as the default strategy
Regarding "Create a merge commit":
At times, I find this option valuable too. Consider a PR that cleans up a
test suite. As part of that cleanup I might re-order the test cases. As
re-ordering produces a noisy diff, I usually isolate it in its own commit.
I would not want to merge this commit with the others as that would lead to
a commit with a very incomprehensible diff. So in this case I would favor
leaving the commits separate and doing an actual merge to group them
together in the global history.
Am Fr., 4. Okt. 2019 um 17:03 Uhr schrieb julio cesar sanchez <
jcesarmobile@gmail.com>:
> Sorry if it wasn't clear, I said I was leaving the protecting master branch
> out of the vote.
>
> Looks like we all agree on using "Squash and merge" as default, unless it
> makes more sense to use one of the other options.
>
> El jue., 3 oct. 2019 a las 3:43, Bryan Ellis (<er...@apache.org>)
> escribió:
>
> > -1 for protected master branches.
> > Protecting a branch is a great idea except it does not work with our
> > current workflow process. COHO commits directly onto the master branch
> for
> > releases. We would have to spend more time planning and changing our
> entire
> > current workflow process to eliminate direct commits if we wanted to
> > protect branches. There is alternative such keeping master open for
> direct
> > commits and development while creating a protected "production" branch.
> > Anyways this part of the discussion is off-topic and could be another
> > discussion... Anyways, stated above I am downvoting protected branches
> for
> > now.
> >
> > +1 for "Squash and merge"
> > Makes a nice single commit with the PR number and without the extra merge
> > commit.
> >
> > +1 for "Rebase and merge"
> > There are use cases where this can work perfectly.
> > For example, Cordova-Electron has a `draft-2.0.0` branch which is
> targeting
> > the next major release. Major PRs were merged into that branch with
> "Squash
> > and merge". This means all PRs have nice PR# information in the title.
> > A PR will later be created to merge this branch onto master. "Rebase and
> > merge" will be used and will not create the merge commit message and will
> > not squash.
> >
> > -1 for "Create a merge commit"
> > Not in favor of the extra merge commit. I think in most cases one PR
> should
> > focus on one task so that means it could be squashed when there are
> > multiple commits. If "Create a merge commit" is used, each commit will be
> > merged and does not contain the PR# in the title. When creating release
> > notes, I need to manually review those commits to identify what PR it
> came
> > from to include the PR linking. Some times, depending on if they are all
> > related commits, I need to manually group them and create a meaningful
> > title for the release notes which I would prefer to have been done
> > beforehand.
> >
> >
> > On Wed, Oct 2, 2019 at 12:51 AM Jesse <pu...@gmail.com> wrote:
> >
> > > -1 for protected master branches, we are a small group of committers
> and
> > > don't need rules to keep us honest.
> > > Protecting master would involve infra, as we cannot manage the minutia
> in
> > > github. I think we all do this anyway except for rare occasions.
> > >
> > > +1 for squash and merge, as long as the meaningful individual commit
> > > messages get consolidated into the 1 commit.
> > >
> > > On Tue, Oct 1, 2019 at 8:36 AM Norman Breau <no...@normanbreau.com>
> > > wrote:
> > >
> > > > +1 to protect the master branch.
> > > >
> > > > Forcing PR will help organize commits if we need to go back in
> > > > time to determine the reason why a change was made as the
> > > > commit in github will show the corresponding PR. Which will
> > > > (hopefully) be properly filled out with context and motivation,
> > > > as well as the issues that the PR resolves.
> > > >
> > > > +1 for "squash + merge" as a default strategy.
> > > >
> > > > I feel like most of the time having all the individual commits that
> > > > makes up a PR is not really necessary. Most of the time it's
> > > > bloated with tweaks fixing errors that was introduced during the
> > > > development of the PR or perhaps refactoring for code style, etc.
> > > > If there are instances where squash shouldn't be used, that can
> > > > be decided on a per-case basis in my opinion.
> > > >
> > > > On 2019-10-01 10:37 a.m., Chris Brody wrote:
> > > > > I would agree that "squash + merge" should be used in *most* cases,
> > > > > and I think there is no dispute on this point.
> > > > >
> > > > > In the few cases where there are too many things for a "squash +
> > > > > merge" commit, and we have the changes down to a limited number of
> > > > > clean, sensible commits, then I would favor merging with a merge
> > > > > commit that shows the PR number.
> > > > >
> > > > > My issue with "rebase and merge" is that the commit history would
> not
> > > > > show the PR number.
> > > > >
> > > > > I think that having the commits show the PR number would make it a
> > > > > little easier for whoever makes the release to make the release
> notes
> > > > > with the PR numbers.
> > > > >
> > > > > Are there any recent examples of "a lot of commits from the same
> PR
> > > > > with meaningless commit messages when changes were requested"?
> > > > >
> > > > > Maybe off-topic: I wonder if we should look for multiple committers
> > to
> > > > > approve an external contribution before merging?
> > > > >
> > > > >
> > > > >
> > > > > On Tue, Oct 1, 2019 at 7:18 AM julio cesar sanchez
> > > > > <jc...@gmail.com> wrote:
> > > > >> Last year, Jan started a thread with different topics and one of
> > them
> > > > was
> > > > >> to have a merge convention. I copy the text:
> > > > >>
> > > > >>> 3. Merge Conventions / Protected Branch:
> > > > >>> Connected to all that is my suggestion to protect the `master`
> > branch
> > > > so
> > > > >> that by default nobody can commit there - all changes have to be
> > made
> > > > via
> > > > >> Pull Requests. Pull Requests are by default merged via the
> "Squash +
> > > > Merge"
> > > > >> button / functionality so that all commits are squashed into one
> > clean
> > > > >> commit per change. This also enforces the commit message
> structure I
> > > > posted
> > > > >> above. (Of course committers can choose to _not_ use Squash +
> Merge
> > if
> > > > >> appropriate for the PR - e.g. when cherry picking commits from a
> > > release
> > > > >> branch or similar).
> > > > >>
> > > > >>> What do you think about this suggestion?
> > > > >> Looks like we didn't agree on anything, but can we agree now?
> > > > >>
> > > > >> I've checked a few repos and some of them have a lot of commits
> from
> > > the
> > > > >> same PR with meaningless commit messages when changes were
> > requested,
> > > > plus
> > > > >> the ugly "merge PR ### from YYY" that makes the commit history
> hard
> > to
> > > > >> follow and hard to cherry pick if needed.
> > > > >>
> > > > >> Since I'm not sure if we can protect branches, I'll focus only on
> > the
> > > > merge
> > > > >> convention.
> > > > >>
> > > > >> Can we all agree on using the "squash + merge" for user PRs,
> unless
> > we
> > > > >> think the different commits makes sense, in this case we should
> try
> > > the
> > > > >> "rebase and merge" button.
> > > > >>
> > > > >> I vote +1
> > > > >
> ---------------------------------------------------------------------
> > > > > To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > > > > For additional commands, e-mail: dev-help@cordova.apache.org
> > > > >
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > > > For additional commands, e-mail: dev-help@cordova.apache.org
> > > >
> > > >
> > >
> >
>
Re: vote: PR merge convention
Posted by julio cesar sanchez <jc...@gmail.com>.
Sorry if it wasn't clear, I said I was leaving the protecting master branch
out of the vote.
Looks like we all agree on using "Squash and merge" as default, unless it
makes more sense to use one of the other options.
El jue., 3 oct. 2019 a las 3:43, Bryan Ellis (<er...@apache.org>) escribió:
> -1 for protected master branches.
> Protecting a branch is a great idea except it does not work with our
> current workflow process. COHO commits directly onto the master branch for
> releases. We would have to spend more time planning and changing our entire
> current workflow process to eliminate direct commits if we wanted to
> protect branches. There is alternative such keeping master open for direct
> commits and development while creating a protected "production" branch.
> Anyways this part of the discussion is off-topic and could be another
> discussion... Anyways, stated above I am downvoting protected branches for
> now.
>
> +1 for "Squash and merge"
> Makes a nice single commit with the PR number and without the extra merge
> commit.
>
> +1 for "Rebase and merge"
> There are use cases where this can work perfectly.
> For example, Cordova-Electron has a `draft-2.0.0` branch which is targeting
> the next major release. Major PRs were merged into that branch with "Squash
> and merge". This means all PRs have nice PR# information in the title.
> A PR will later be created to merge this branch onto master. "Rebase and
> merge" will be used and will not create the merge commit message and will
> not squash.
>
> -1 for "Create a merge commit"
> Not in favor of the extra merge commit. I think in most cases one PR should
> focus on one task so that means it could be squashed when there are
> multiple commits. If "Create a merge commit" is used, each commit will be
> merged and does not contain the PR# in the title. When creating release
> notes, I need to manually review those commits to identify what PR it came
> from to include the PR linking. Some times, depending on if they are all
> related commits, I need to manually group them and create a meaningful
> title for the release notes which I would prefer to have been done
> beforehand.
>
>
> On Wed, Oct 2, 2019 at 12:51 AM Jesse <pu...@gmail.com> wrote:
>
> > -1 for protected master branches, we are a small group of committers and
> > don't need rules to keep us honest.
> > Protecting master would involve infra, as we cannot manage the minutia in
> > github. I think we all do this anyway except for rare occasions.
> >
> > +1 for squash and merge, as long as the meaningful individual commit
> > messages get consolidated into the 1 commit.
> >
> > On Tue, Oct 1, 2019 at 8:36 AM Norman Breau <no...@normanbreau.com>
> > wrote:
> >
> > > +1 to protect the master branch.
> > >
> > > Forcing PR will help organize commits if we need to go back in
> > > time to determine the reason why a change was made as the
> > > commit in github will show the corresponding PR. Which will
> > > (hopefully) be properly filled out with context and motivation,
> > > as well as the issues that the PR resolves.
> > >
> > > +1 for "squash + merge" as a default strategy.
> > >
> > > I feel like most of the time having all the individual commits that
> > > makes up a PR is not really necessary. Most of the time it's
> > > bloated with tweaks fixing errors that was introduced during the
> > > development of the PR or perhaps refactoring for code style, etc.
> > > If there are instances where squash shouldn't be used, that can
> > > be decided on a per-case basis in my opinion.
> > >
> > > On 2019-10-01 10:37 a.m., Chris Brody wrote:
> > > > I would agree that "squash + merge" should be used in *most* cases,
> > > > and I think there is no dispute on this point.
> > > >
> > > > In the few cases where there are too many things for a "squash +
> > > > merge" commit, and we have the changes down to a limited number of
> > > > clean, sensible commits, then I would favor merging with a merge
> > > > commit that shows the PR number.
> > > >
> > > > My issue with "rebase and merge" is that the commit history would not
> > > > show the PR number.
> > > >
> > > > I think that having the commits show the PR number would make it a
> > > > little easier for whoever makes the release to make the release notes
> > > > with the PR numbers.
> > > >
> > > > Are there any recent examples of "a lot of commits from the same PR
> > > > with meaningless commit messages when changes were requested"?
> > > >
> > > > Maybe off-topic: I wonder if we should look for multiple committers
> to
> > > > approve an external contribution before merging?
> > > >
> > > >
> > > >
> > > > On Tue, Oct 1, 2019 at 7:18 AM julio cesar sanchez
> > > > <jc...@gmail.com> wrote:
> > > >> Last year, Jan started a thread with different topics and one of
> them
> > > was
> > > >> to have a merge convention. I copy the text:
> > > >>
> > > >>> 3. Merge Conventions / Protected Branch:
> > > >>> Connected to all that is my suggestion to protect the `master`
> branch
> > > so
> > > >> that by default nobody can commit there - all changes have to be
> made
> > > via
> > > >> Pull Requests. Pull Requests are by default merged via the "Squash +
> > > Merge"
> > > >> button / functionality so that all commits are squashed into one
> clean
> > > >> commit per change. This also enforces the commit message structure I
> > > posted
> > > >> above. (Of course committers can choose to _not_ use Squash + Merge
> if
> > > >> appropriate for the PR - e.g. when cherry picking commits from a
> > release
> > > >> branch or similar).
> > > >>
> > > >>> What do you think about this suggestion?
> > > >> Looks like we didn't agree on anything, but can we agree now?
> > > >>
> > > >> I've checked a few repos and some of them have a lot of commits from
> > the
> > > >> same PR with meaningless commit messages when changes were
> requested,
> > > plus
> > > >> the ugly "merge PR ### from YYY" that makes the commit history hard
> to
> > > >> follow and hard to cherry pick if needed.
> > > >>
> > > >> Since I'm not sure if we can protect branches, I'll focus only on
> the
> > > merge
> > > >> convention.
> > > >>
> > > >> Can we all agree on using the "squash + merge" for user PRs, unless
> we
> > > >> think the different commits makes sense, in this case we should try
> > the
> > > >> "rebase and merge" button.
> > > >>
> > > >> I vote +1
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > > > For additional commands, e-mail: dev-help@cordova.apache.org
> > > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > > For additional commands, e-mail: dev-help@cordova.apache.org
> > >
> > >
> >
>
Re: vote: PR merge convention
Posted by Bryan Ellis <er...@apache.org>.
-1 for protected master branches.
Protecting a branch is a great idea except it does not work with our
current workflow process. COHO commits directly onto the master branch for
releases. We would have to spend more time planning and changing our entire
current workflow process to eliminate direct commits if we wanted to
protect branches. There is alternative such keeping master open for direct
commits and development while creating a protected "production" branch.
Anyways this part of the discussion is off-topic and could be another
discussion... Anyways, stated above I am downvoting protected branches for
now.
+1 for "Squash and merge"
Makes a nice single commit with the PR number and without the extra merge
commit.
+1 for "Rebase and merge"
There are use cases where this can work perfectly.
For example, Cordova-Electron has a `draft-2.0.0` branch which is targeting
the next major release. Major PRs were merged into that branch with "Squash
and merge". This means all PRs have nice PR# information in the title.
A PR will later be created to merge this branch onto master. "Rebase and
merge" will be used and will not create the merge commit message and will
not squash.
-1 for "Create a merge commit"
Not in favor of the extra merge commit. I think in most cases one PR should
focus on one task so that means it could be squashed when there are
multiple commits. If "Create a merge commit" is used, each commit will be
merged and does not contain the PR# in the title. When creating release
notes, I need to manually review those commits to identify what PR it came
from to include the PR linking. Some times, depending on if they are all
related commits, I need to manually group them and create a meaningful
title for the release notes which I would prefer to have been done
beforehand.
On Wed, Oct 2, 2019 at 12:51 AM Jesse <pu...@gmail.com> wrote:
> -1 for protected master branches, we are a small group of committers and
> don't need rules to keep us honest.
> Protecting master would involve infra, as we cannot manage the minutia in
> github. I think we all do this anyway except for rare occasions.
>
> +1 for squash and merge, as long as the meaningful individual commit
> messages get consolidated into the 1 commit.
>
> On Tue, Oct 1, 2019 at 8:36 AM Norman Breau <no...@normanbreau.com>
> wrote:
>
> > +1 to protect the master branch.
> >
> > Forcing PR will help organize commits if we need to go back in
> > time to determine the reason why a change was made as the
> > commit in github will show the corresponding PR. Which will
> > (hopefully) be properly filled out with context and motivation,
> > as well as the issues that the PR resolves.
> >
> > +1 for "squash + merge" as a default strategy.
> >
> > I feel like most of the time having all the individual commits that
> > makes up a PR is not really necessary. Most of the time it's
> > bloated with tweaks fixing errors that was introduced during the
> > development of the PR or perhaps refactoring for code style, etc.
> > If there are instances where squash shouldn't be used, that can
> > be decided on a per-case basis in my opinion.
> >
> > On 2019-10-01 10:37 a.m., Chris Brody wrote:
> > > I would agree that "squash + merge" should be used in *most* cases,
> > > and I think there is no dispute on this point.
> > >
> > > In the few cases where there are too many things for a "squash +
> > > merge" commit, and we have the changes down to a limited number of
> > > clean, sensible commits, then I would favor merging with a merge
> > > commit that shows the PR number.
> > >
> > > My issue with "rebase and merge" is that the commit history would not
> > > show the PR number.
> > >
> > > I think that having the commits show the PR number would make it a
> > > little easier for whoever makes the release to make the release notes
> > > with the PR numbers.
> > >
> > > Are there any recent examples of "a lot of commits from the same PR
> > > with meaningless commit messages when changes were requested"?
> > >
> > > Maybe off-topic: I wonder if we should look for multiple committers to
> > > approve an external contribution before merging?
> > >
> > >
> > >
> > > On Tue, Oct 1, 2019 at 7:18 AM julio cesar sanchez
> > > <jc...@gmail.com> wrote:
> > >> Last year, Jan started a thread with different topics and one of them
> > was
> > >> to have a merge convention. I copy the text:
> > >>
> > >>> 3. Merge Conventions / Protected Branch:
> > >>> Connected to all that is my suggestion to protect the `master` branch
> > so
> > >> that by default nobody can commit there - all changes have to be made
> > via
> > >> Pull Requests. Pull Requests are by default merged via the "Squash +
> > Merge"
> > >> button / functionality so that all commits are squashed into one clean
> > >> commit per change. This also enforces the commit message structure I
> > posted
> > >> above. (Of course committers can choose to _not_ use Squash + Merge if
> > >> appropriate for the PR - e.g. when cherry picking commits from a
> release
> > >> branch or similar).
> > >>
> > >>> What do you think about this suggestion?
> > >> Looks like we didn't agree on anything, but can we agree now?
> > >>
> > >> I've checked a few repos and some of them have a lot of commits from
> the
> > >> same PR with meaningless commit messages when changes were requested,
> > plus
> > >> the ugly "merge PR ### from YYY" that makes the commit history hard to
> > >> follow and hard to cherry pick if needed.
> > >>
> > >> Since I'm not sure if we can protect branches, I'll focus only on the
> > merge
> > >> convention.
> > >>
> > >> Can we all agree on using the "squash + merge" for user PRs, unless we
> > >> think the different commits makes sense, in this case we should try
> the
> > >> "rebase and merge" button.
> > >>
> > >> I vote +1
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > > For additional commands, e-mail: dev-help@cordova.apache.org
> > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > For additional commands, e-mail: dev-help@cordova.apache.org
> >
> >
>
Re: vote: PR merge convention
Posted by Jesse <pu...@gmail.com>.
-1 for protected master branches, we are a small group of committers and
don't need rules to keep us honest.
Protecting master would involve infra, as we cannot manage the minutia in
github. I think we all do this anyway except for rare occasions.
+1 for squash and merge, as long as the meaningful individual commit
messages get consolidated into the 1 commit.
On Tue, Oct 1, 2019 at 8:36 AM Norman Breau <no...@normanbreau.com> wrote:
> +1 to protect the master branch.
>
> Forcing PR will help organize commits if we need to go back in
> time to determine the reason why a change was made as the
> commit in github will show the corresponding PR. Which will
> (hopefully) be properly filled out with context and motivation,
> as well as the issues that the PR resolves.
>
> +1 for "squash + merge" as a default strategy.
>
> I feel like most of the time having all the individual commits that
> makes up a PR is not really necessary. Most of the time it's
> bloated with tweaks fixing errors that was introduced during the
> development of the PR or perhaps refactoring for code style, etc.
> If there are instances where squash shouldn't be used, that can
> be decided on a per-case basis in my opinion.
>
> On 2019-10-01 10:37 a.m., Chris Brody wrote:
> > I would agree that "squash + merge" should be used in *most* cases,
> > and I think there is no dispute on this point.
> >
> > In the few cases where there are too many things for a "squash +
> > merge" commit, and we have the changes down to a limited number of
> > clean, sensible commits, then I would favor merging with a merge
> > commit that shows the PR number.
> >
> > My issue with "rebase and merge" is that the commit history would not
> > show the PR number.
> >
> > I think that having the commits show the PR number would make it a
> > little easier for whoever makes the release to make the release notes
> > with the PR numbers.
> >
> > Are there any recent examples of "a lot of commits from the same PR
> > with meaningless commit messages when changes were requested"?
> >
> > Maybe off-topic: I wonder if we should look for multiple committers to
> > approve an external contribution before merging?
> >
> >
> >
> > On Tue, Oct 1, 2019 at 7:18 AM julio cesar sanchez
> > <jc...@gmail.com> wrote:
> >> Last year, Jan started a thread with different topics and one of them
> was
> >> to have a merge convention. I copy the text:
> >>
> >>> 3. Merge Conventions / Protected Branch:
> >>> Connected to all that is my suggestion to protect the `master` branch
> so
> >> that by default nobody can commit there - all changes have to be made
> via
> >> Pull Requests. Pull Requests are by default merged via the "Squash +
> Merge"
> >> button / functionality so that all commits are squashed into one clean
> >> commit per change. This also enforces the commit message structure I
> posted
> >> above. (Of course committers can choose to _not_ use Squash + Merge if
> >> appropriate for the PR - e.g. when cherry picking commits from a release
> >> branch or similar).
> >>
> >>> What do you think about this suggestion?
> >> Looks like we didn't agree on anything, but can we agree now?
> >>
> >> I've checked a few repos and some of them have a lot of commits from the
> >> same PR with meaningless commit messages when changes were requested,
> plus
> >> the ugly "merge PR ### from YYY" that makes the commit history hard to
> >> follow and hard to cherry pick if needed.
> >>
> >> Since I'm not sure if we can protect branches, I'll focus only on the
> merge
> >> convention.
> >>
> >> Can we all agree on using the "squash + merge" for user PRs, unless we
> >> think the different commits makes sense, in this case we should try the
> >> "rebase and merge" button.
> >>
> >> I vote +1
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > For additional commands, e-mail: dev-help@cordova.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> For additional commands, e-mail: dev-help@cordova.apache.org
>
>
Re: vote: PR merge convention
Posted by Norman Breau <no...@normanbreau.com>.
+1 to protect the master branch.
Forcing PR will help organize commits if we need to go back in
time to determine the reason why a change was made as the
commit in github will show the corresponding PR. Which will
(hopefully) be properly filled out with context and motivation,
as well as the issues that the PR resolves.
+1 for "squash + merge" as a default strategy.
I feel like most of the time having all the individual commits that
makes up a PR is not really necessary. Most of the time it's
bloated with tweaks fixing errors that was introduced during the
development of the PR or perhaps refactoring for code style, etc.
If there are instances where squash shouldn't be used, that can
be decided on a per-case basis in my opinion.
On 2019-10-01 10:37 a.m., Chris Brody wrote:
> I would agree that "squash + merge" should be used in *most* cases,
> and I think there is no dispute on this point.
>
> In the few cases where there are too many things for a "squash +
> merge" commit, and we have the changes down to a limited number of
> clean, sensible commits, then I would favor merging with a merge
> commit that shows the PR number.
>
> My issue with "rebase and merge" is that the commit history would not
> show the PR number.
>
> I think that having the commits show the PR number would make it a
> little easier for whoever makes the release to make the release notes
> with the PR numbers.
>
> Are there any recent examples of "a lot of commits from the same PR
> with meaningless commit messages when changes were requested"?
>
> Maybe off-topic: I wonder if we should look for multiple committers to
> approve an external contribution before merging?
>
>
>
> On Tue, Oct 1, 2019 at 7:18 AM julio cesar sanchez
> <jc...@gmail.com> wrote:
>> Last year, Jan started a thread with different topics and one of them was
>> to have a merge convention. I copy the text:
>>
>>> 3. Merge Conventions / Protected Branch:
>>> Connected to all that is my suggestion to protect the `master` branch so
>> that by default nobody can commit there - all changes have to be made via
>> Pull Requests. Pull Requests are by default merged via the "Squash + Merge"
>> button / functionality so that all commits are squashed into one clean
>> commit per change. This also enforces the commit message structure I posted
>> above. (Of course committers can choose to _not_ use Squash + Merge if
>> appropriate for the PR - e.g. when cherry picking commits from a release
>> branch or similar).
>>
>>> What do you think about this suggestion?
>> Looks like we didn't agree on anything, but can we agree now?
>>
>> I've checked a few repos and some of them have a lot of commits from the
>> same PR with meaningless commit messages when changes were requested, plus
>> the ugly "merge PR ### from YYY" that makes the commit history hard to
>> follow and hard to cherry pick if needed.
>>
>> Since I'm not sure if we can protect branches, I'll focus only on the merge
>> convention.
>>
>> Can we all agree on using the "squash + merge" for user PRs, unless we
>> think the different commits makes sense, in this case we should try the
>> "rebase and merge" button.
>>
>> I vote +1
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> For additional commands, e-mail: dev-help@cordova.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
Re: vote: PR merge convention
Posted by Chris Brody <ch...@gmail.com>.
I would agree that "squash + merge" should be used in *most* cases,
and I think there is no dispute on this point.
In the few cases where there are too many things for a "squash +
merge" commit, and we have the changes down to a limited number of
clean, sensible commits, then I would favor merging with a merge
commit that shows the PR number.
My issue with "rebase and merge" is that the commit history would not
show the PR number.
I think that having the commits show the PR number would make it a
little easier for whoever makes the release to make the release notes
with the PR numbers.
Are there any recent examples of "a lot of commits from the same PR
with meaningless commit messages when changes were requested"?
Maybe off-topic: I wonder if we should look for multiple committers to
approve an external contribution before merging?
On Tue, Oct 1, 2019 at 7:18 AM julio cesar sanchez
<jc...@gmail.com> wrote:
>
> Last year, Jan started a thread with different topics and one of them was
> to have a merge convention. I copy the text:
>
> > 3. Merge Conventions / Protected Branch:
>
> > Connected to all that is my suggestion to protect the `master` branch so
> that by default nobody can commit there - all changes have to be made via
> Pull Requests. Pull Requests are by default merged via the "Squash + Merge"
> button / functionality so that all commits are squashed into one clean
> commit per change. This also enforces the commit message structure I posted
> above. (Of course committers can choose to _not_ use Squash + Merge if
> appropriate for the PR - e.g. when cherry picking commits from a release
> branch or similar).
>
> > What do you think about this suggestion?
>
> Looks like we didn't agree on anything, but can we agree now?
>
> I've checked a few repos and some of them have a lot of commits from the
> same PR with meaningless commit messages when changes were requested, plus
> the ugly "merge PR ### from YYY" that makes the commit history hard to
> follow and hard to cherry pick if needed.
>
> Since I'm not sure if we can protect branches, I'll focus only on the merge
> convention.
>
> Can we all agree on using the "squash + merge" for user PRs, unless we
> think the different commits makes sense, in this case we should try the
> "rebase and merge" button.
>
> I vote +1
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org