Hi everyone,
We had a good conversation going here. Maybe we can continue it, get some level
of reasonable consensus, and implement it (if, in fact, the consensus is a
change from what we currently have).
My suggested approach is the following:
Before you create a PR, squash all applicable commits to make it more readable
for reviewers. Once reviews start coming in and you start making changes, push
new commits on top of the prior ones (do not squash at this point). This will
make it easier for reviewers to confirm that you and they are on the same page
with regards to what was changed. When you need to draw in changes from the
base branch, rebase your commits on top of it. When the PR is given a LGTM by
2+ reviewers and passed the necessary regression tests, it should be squashed
and then merged. I see the evolution of commits during the life of the PR as a
temporary sandbox of history that is no longer required once the PR has been
completed.
I think that process should cover the vast majority of our PRs.
There are usually some exceptions to the rule, however. When this happens,
discuss your situation with the reviewers and bring any concerns to the mailing
list before deviating from the standard process.
Thoughts?
Mike
On 1/15/18, 1:47 PM, "Rene Moser" <m...@renemoser.net> wrote:
On 01/15/2018 09:06 PM, Rafael Weingärtner wrote:
> Daan,
>
> Now that master is open for merges again, can we get a feedback here? It
> might be interesting to find a consensus and a standardize way of working
> for everybody before we start merging things in master again …
+1 to allow merge commits on master branch to keep history of a series
of patches when they help to understand the change.
René
