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 <purplecabb...@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 <nor...@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 > > > > <jcesarmob...@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-unsubscr...@cordova.apache.org > > > > For additional commands, e-mail: dev-h...@cordova.apache.org > > > > > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org > > > For additional commands, e-mail: dev-h...@cordova.apache.org > > > > > > > > >
