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 2020/05/02 11:38:15 UTC

Re: vote: PR merge convention

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 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