Re: [Jflex-devel] Branch protection

[Régis D. <re...@de...>]

Hello Gerwin,

Le jeu. 9 nov. 2017 à 23:10, Gerwin Klein <ge...@do...> a écrit :

> On 10.11.2017, at 00:46, Régis Décamps via Jflex-devel <
> jfl...@li...> wrote:
>
> Hello all,
>
> I've accidentally submitted to master a couple of times.
>
> Github has a nice feature to protect branches. How do you feel about protecting
> master <https://github.com/jflex-de/jflex/settings/branches/master>?
>
> I propose to restrict master to PR only ; and we can even enforce CI status
> check as well
> <https://help.github.com/articles/enabling-required-status-checks/>.
>
>
> I’d be happy with that.
>

Just to close the loop, I think I feel very happy with the protection of
the branches.

Also the status check from Travis comes fast enough and provides a nice
safety bet. I feel almost in the same environment than at work.

I tentatively enabled a more restrictive "Require branches to be up to date
before merging". It was annoying. You have to press the merge button and
wait again. If you submit multiple things, it's really annoying that you
need to press "Merge". I think it would be nice if github had auto-merge,
but until then,I dislike this protection.

 Now, as you can read in #410 <https://github.com/jflex-de/jflex/pull/410> I'm
experimenting with a few code quality services. I really like Semmle
lgtm.com <lgtm.comhttps://lgtm.com/projects/g/jflex-de/jflex> the most so
far.


> Also, I personally feel that the cleanest way to integrate the branch is
> to "Squash and merge", but that's a slightly different topic.
>
>
> The merge pattern depends for me. I like topical commits that pertain to
> one specific thing. A pull request is often just one of these, then squash
> and merge is good, but it’s also reasonably common that there are multiple
> things on one theme that each make sense on their own in a PR, and then I’d
> usually prefer rebase and merge. I do tend towards smaller commits myself,
> but I don’t think everyone has to do that.
>
> The advantage of smaller topical commits is that you can isolate features
> and still revert/reorder things afterwards if necessary, e.g. when you
> later figure out that some feature doesn’t quite interact so well with the
> rest and you have to remove or change it.
>

I still prefer the squash ; I believe a PR should be topical in the first
place.

I really dislike the standard merge commit. It creates a complex history
that shows the temporary feature branch.[image: Screen Shot 2018-10-07 at
11.57.50.png]
(I think merge makes more sense in case you merge a maintenance branch into
master/devel branch).

If the initial PR has only one commit or if the few commits are very
topical, I like rebase and merge.

In all other situations, squash is my favourite strategy.

Régis
-- 

-- 
Régis