search

GreenPix / meta

This is meta!
http://greenpix.github.io/meta
3 stars 0 forks source link

PR worflow #13

Open Nemikolh opened 7 years ago

Nemikolh commented 7 years ago

@lethom suggested to use the squash & merge feature of github in GreenPix/dilia#18:

Note: rather than merge using the default option (and create a merge commit), we should use the squash & merge feature of github.

So far we haven't talked about whether we should use this option. Personally I don't like having only PR of a single commit because I prefer to have a PR with a nice story around it that will quite often need several commits rather than a single one. However, preserving that with the default option in github results in having an additional commit created once they get merged onto master.

We could obviously avoid that by doing manual merge and simply do a rebase against the master branch and fast forward the repo manually. However it remove some of the benefit from using the github interface. Also, to do a manual fast forward of the master branch, we would have to unprotect the master branch and allow us to push on the master branch which is not ideal either. Especially when we require the CI to pass before any form of merge can be done.

I'm split on this, but my opinion is that merge commit created by github are okay and unless the PR is made of a single commit, in which case the squash & merge feature from github is fine, I would stick to use the merge option.

@lethom, @Vaelden opinions?

lthms commented 7 years ago

I'm split on this, but my opinion is that merge commit created by github are okay and unless the PR is made of a single commit, in which case the squash & merge feature from github is fine, I would stick to use the merge option.

Well actually, I only proposed this solution because the PR was only 1 commit long. In other words, I totally agree with you.

Nemikolh commented 7 years ago

Oh ! My bad, I thought this was a general comment rather than specific to the PR.

I'll leave the issue open until we get feedback from @Vaelden. :-)

lthms commented 7 years ago

If he can find its way out from VR d: