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 <gandhiraja...@gmail.com> escribió: > + 1 for squash and merge as it makes the repo history cleaner. > > On Sat, Oct 5, 2019 at 8:34 PM <raphine...@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 < > > jcesarmob...@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 <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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > Regards, > Gandhi > > "The best way to find urself is to lose urself in the service of others > !!!" >
