llvm dev - Mar 2019 - [cfe-dev] [GitHub] RFC: Enforcing no merge commit policy

I think we definitely will want to support github PRs, at the very least as
an _option_, even if we continue running/preferring phabricator.

Github PRs are how everyone who is not already super-involved in the llvm
project is going to want to contribute changes, and we ought to be as
welcoming as possible to such users.

On Tue, Mar 19, 2019 at 3:15 PM Roman Lebedev via cfe-dev <
cfe-dev at lists.llvm.org> wrote:
> On Tue, Mar 19, 2019 at 10:00 PM Tom Stellard via cfe-dev
> <cfe-dev at lists.llvm.org> wrote:
> >
> > Hi,
> >
> > I would like to follow up on the previous thread[1], where there was a
> consensus
> > to disallow merge commits in the llvm github repository, and start a
> discussion
> > about how we should enforce this policy.
> >
> > Unfortunately, GitHub does not provide a convenient way to fully
enforce
> this policy.
> > We can enforce it for pull requests, but not for direct pushes to the
> master branch,
> > so we will have to come up with our own solution if we want to
> completely prevent
> > merge commits.  I've spent some time looking into this and here
are a few
> > options that I've come up with (If you are a GitHub expert and
have other
> > suggestions, please let us know):
> >
> > 1. Have either a status check or use the "Rebase and Merge"
policy for
> pull requests
> > to disallow merge commits.  Disable direct pushes to the master branch
> and update the
> > git-llvm script to create a pull request when a developer does `git
llvm
> push` and
> > then have it automatically merged if there are no merge commits.
> >
> > 2. Enforce no merge commits for pull requests, but sill allow
developers
> to push directly
> > to master without checking for merge requests.  This is essentially a
> best effort
> > approach where we avoid having to implement our own custom work-flow
for
> committing,
> > while accepting the possibility that someone might accidentally push a
> merge commit.
> >
> > 3. Disable direct pushes to master, don't update the git-llvm
script and
> require all
> > developers to use pull requests, where this policy will be enforced.
> >
> > Which option do you prefer?
> All these options include at least partial usage of/switch to github
pr's.
> I don't think that was discussed before. (FWIW i'm opposed to
that.)
>
> Is there a fourth option - ask github whether they could make an exception
> for LLVM to use server-side hooks? They are available in github enterprise.
>
> > -Tom
> Roman.
>
> > [1] http://lists.llvm.org/pipermail/llvm-dev/2019-January/129723.html
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190319/9144f1b6/attachment.html>

Honestly I'm looking forward to GitHub's interface here.

On Tue, Mar 19, 2019, 1:25 PM James Y Knight via lldb-dev <
lldb-dev at lists.llvm.org> wrote:
> I think we definitely will want to support github PRs, at the very least
> as an _option_, even if we continue running/preferring phabricator.
>
> Github PRs are how everyone who is not already super-involved in the llvm
> project is going to want to contribute changes, and we ought to be as
> welcoming as possible to such users.
>
> On Tue, Mar 19, 2019 at 3:15 PM Roman Lebedev via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>> On Tue, Mar 19, 2019 at 10:00 PM Tom Stellard via cfe-dev
>> <cfe-dev at lists.llvm.org> wrote:
>> >
>> > Hi,
>> >
>> > I would like to follow up on the previous thread[1], where there
was a
>> consensus
>> > to disallow merge commits in the llvm github repository, and start
a
>> discussion
>> > about how we should enforce this policy.
>> >
>> > Unfortunately, GitHub does not provide a convenient way to fully
>> enforce this policy.
>> > We can enforce it for pull requests, but not for direct pushes to
the
>> master branch,
>> > so we will have to come up with our own solution if we want to
>> completely prevent
>> > merge commits.  I've spent some time looking into this and
here are a
>> few
>> > options that I've come up with (If you are a GitHub expert and
have
>> other
>> > suggestions, please let us know):
>> >
>> > 1. Have either a status check or use the "Rebase and
Merge" policy for
>> pull requests
>> > to disallow merge commits.  Disable direct pushes to the master
branch
>> and update the
>> > git-llvm script to create a pull request when a developer does
`git
>> llvm push` and
>> > then have it automatically merged if there are no merge commits.
>> >
>> > 2. Enforce no merge commits for pull requests, but sill allow
>> developers to push directly
>> > to master without checking for merge requests.  This is
essentially a
>> best effort
>> > approach where we avoid having to implement our own custom
work-flow
>> for committing,
>> > while accepting the possibility that someone might accidentally
push a
>> merge commit.
>> >
>> > 3. Disable direct pushes to master, don't update the git-llvm
script
>> and require all
>> > developers to use pull requests, where this policy will be
enforced.
>> >
>> > Which option do you prefer?
>> All these options include at least partial usage of/switch to github
pr's.
>> I don't think that was discussed before. (FWIW i'm opposed to
that.)
>>
>> Is there a fourth option - ask github whether they could make an
exception
>> for LLVM to use server-side hooks? They are available in github
>> enterprise.
>>
>> > -Tom
>> Roman.
>>
>> > [1]
http://lists.llvm.org/pipermail/llvm-dev/2019-January/129723.html
>> > _______________________________________________
>> > cfe-dev mailing list
>> > cfe-dev at lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>
> _______________________________________________
> lldb-dev mailing list
> lldb-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190319/f4a8c474/attachment.html>

> Github PRs are how everyone who is not already super-involved in the llvm
project is going to want to contribute changes, and we ought to be as welcoming
as possible to such users.
Still we'd need some policy & checks here. Say, what if someone will
issue a PR to LLVM 4.0 branch? With clear code style violations, etc.

-- 
With best regards, Anton Korobeynikov
Department of Statistical Modelling, Saint Petersburg State University

Then we should not accept it. What if someone did the same on a phabricator
review?

On Wed, Mar 20, 2019 at 2:39 AM Anton Korobeynikov <anton at
korobeynikov.info>
wrote:
> > Github PRs are how everyone who is not already super-involved in the
> llvm project is going to want to contribute changes, and we ought to be as
> welcoming as possible to such users.
> Still we'd need some policy & checks here. Say, what if someone
will
> issue a PR to LLVM 4.0 branch? With clear code style violations, etc.
>
> --
> With best regards, Anton Korobeynikov
> Department of Statistical Modelling, Saint Petersburg State University
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190320/6d670b6b/attachment.html>