Tom Stellard via llvm-dev
2019-Mar-19 18:59 UTC
[llvm-dev] [GitHub] RFC: Enforcing no merge commit policy
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? -Tom [1] http://lists.llvm.org/pipermail/llvm-dev/2019-January/129723.html
Roman Lebedev via llvm-dev
2019-Mar-19 19:14 UTC
[llvm-dev] [cfe-dev] [GitHub] RFC: Enforcing no merge commit policy
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.> -TomRoman.> [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
JF Bastien via llvm-dev
2019-Mar-19 19:48 UTC
[llvm-dev] [cfe-dev] [GitHub] RFC: Enforcing no merge commit policy
> On Mar 19, 2019, at 12:14 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 <mailto: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.It sounds like GitHub folks are very eager to help the LLVM project: https://twitter.com/natfriedman/status/1086470665832607746 <https://twitter.com/natfriedman/status/1086470665832607746> Server-side hooks seem like a good solution for what’s being discussed. We should ask GitHub about it.>> -Tom > Roman. > >> [1] http://lists.llvm.org/pipermail/llvm-dev/2019-January/129723.html <http://lists.llvm.org/pipermail/llvm-dev/2019-January/129723.html> >> _______________________________________________ >> cfe-dev mailing list >> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev <https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev> > _______________________________________________ > cfe-dev mailing list > cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev <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/46e74ae0/attachment.html>
James Y Knight via llvm-dev
2019-Mar-19 20:24 UTC
[llvm-dev] [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>
David Greene via llvm-dev
2019-Mar-20 16:58 UTC
[llvm-dev] [GitHub] RFC: Enforcing no merge commit policy
Tom Stellard via llvm-dev <llvm-dev at lists.llvm.org> writes:> 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.This seems to be the least disruptive to existing developers while maintaining the invariant we want. It has the advantage of potentially being extended in the future to add additional criteria for merges (e.g. "it builds" or "it passes this set of tests").> 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.To me this is the least desirable. I'd prefer we have one way of getting changes into the repository.> 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.I am completely fine with this. It's friendlier to new contributors. That said, I assume with #1 we wouldn't prevent users from git push-ing their local branch to GitHub (i.e. not using git llvm push) and manually opening a PR, so either #1 or #3 works for new developers.> Which option do you prefer?Maybe a slight preference for #1 as being less disruptive than #3 but I really don't care either way. -David
Zachary Turner via llvm-dev
2019-Mar-20 17:41 UTC
[llvm-dev] [lldb-dev] [GitHub] RFC: Enforcing no merge commit policy
On Tue, Mar 19, 2019 at 12:00 PM Tom Stellard via lldb-dev < lldb-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. > >Why isn't this enforceable with a server-side pre-receive hook? -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190320/ee7354de/attachment.html>
Tom Stellard via llvm-dev
2019-Mar-20 18:19 UTC
[llvm-dev] [lldb-dev] [GitHub] RFC: Enforcing no merge commit policy
On 03/20/2019 10:41 AM, Zachary Turner wrote:> > > On Tue, Mar 19, 2019 at 12:00 PM Tom Stellard via lldb-dev <lldb-dev at lists.llvm.org <mailto:lldb-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. > > > Why isn't this enforceable with a server-side pre-receive hook?GitHub[1] only supports pre-receive hooks in the 'Enterprise Server' plan, which is for self-hosted github instances. -Tom [1] https://github.com/pricing
Brian Cain via llvm-dev
2019-Mar-21 02:38 UTC
[llvm-dev] [cfe-dev] [GitHub] RFC: Enforcing no merge commit policy
On Tue, Mar 19, 2019 at 2: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? > > >I don't think I have a preference regarding merge commits but having a status check where some subset of build + test takes place is a really good idea, IMO. It would be great if it were applied for everyone but if that causes too many problems, I would settle for opt-in. Preferably not all-or-nothing on the check. Regarding the scope of the check (boldly assuming one would be in place): the more, the better (so long as it's stable and tolerable to the team). Some dev teams that use GH have a bot that optimistically batch builds-and-tests commits and when failures inevitably happen, contributors are encouraged to triage and re-enqueue their PR for being built if they can surmise the failure is not due to their change. When contributing changes upstream, it takes away some of the stress/challenge if you have some independent automaton verify that your change doesn't cause regressions. As it stands I feel like I should be on standby for 24 to 72 hours after the commit in order to investigate/revert if my change causes someone problems. I realize that it's prudent to limit the scope such that the checks don't create an enormous backlog. -- -Brian -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190320/6f5f6fe2/attachment.html>
Tom Stellard via llvm-dev
2019-Apr-08 23:32 UTC
[llvm-dev] [Openmp-dev] [GitHub] RFC: Enforcing no merge commit policy
On 03/19/2019 11:59 AM, Tom Stellard via Openmp-dev 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? >Thanks everyone for responding to this thread. Based on the responses, the preferred solution is none of these options and instead having a server-side check to prevent merge commits. I have been in contact with someone at GitHub who is looking into this for us to see if this would be possible. As a backup plan, I am going to look into implementing option #1, but instead of having git-llvm create a pull request, it would push first to a staging branch and then push to master if a 'no-merge commit' status checks pass. This is essentially the same flow as using pull requests, but without all the pull request noise. The reason to go #1 as a backup is that it preserves the status quo we have now where developers commit using git-llvm and based on the limited feedback seems like the preferred option of these 3. - Tom> -Tom > > [1] http://lists.llvm.org/pipermail/llvm-dev/2019-January/129723.html > _______________________________________________ > Openmp-dev mailing list > Openmp-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/openmp-dev >
Maybe Matching Threads
- [cfe-dev] [lldb-dev] [GitHub] RFC: Enforcing no merge commit policy
- [cfe-dev] [lldb-dev] [GitHub] RFC: Enforcing no merge commit policy
- [cfe-dev] [lldb-dev] [GitHub] RFC: Enforcing no merge commit policy
- [GitHub] RFC: Enforcing no merge commit policy
- [cfe-dev] [Github] RFC: linear history vs merge commits