+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
