protected branches on GH

Hey all,

GH now lets you protect branches which prevents people from force-pushing
or deleting those branches;
https://github.com/matplotlib/matplotlib/settings/branches.

I have protected our 'production' branches.

There is also an option to require status checks to pass before merging to
a given branch, but that would break our current cherry-pick workflow and
mean that we could not use our judgement to over-rule travis.ci :wink:

Tom
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/matplotlib-devel/attachments/20151107/2d7b47de/attachment.html>

It looks like you can set it so that the status checks are enforced on
normal users, but administrators are allowed to ignore it:
  Managing a branch protection rule - GitHub Docs

What I really want to know is whether it prevents simply pushing
directly to the branch. That would be really awesome because it would
mean you could hand out commit bits like candy without worrying that
unreviewed code could creep into the repo... but there's no indication
of this anywhere. (OTOH if direct pushes *are* still allowed, then
that also provides a method for circumventing the status checks when
desired -- just do a manual merge and push from the command line.)

-n

路路路

On Fri, Nov 6, 2015 at 8:45 PM, Thomas Caswell <tcaswell at gmail.com> wrote:

Hey all,

GH now lets you protect branches which prevents people from force-pushing or
deleting those branches;
https://github.com/matplotlib/matplotlib/settings/branches.

I have protected our 'production' branches.

There is also an option to require status checks to pass before merging to a
given branch, but that would break our current cherry-pick workflow and mean
that we could not use our judgement to over-rule travis.ci :wink:

--
Nathaniel J. Smith -- http://vorpus.org

We use it here at work. Protected branches are just that--you CANNOT push a
commit (i.e. a hash) until it has passed the checks. One note, though, is
that the commit must have passed the checks running on top of *current*
master. That means if master changes (i.e. you merge another PR), you have
to update the PR (you can use the web button to merge master into the
branch, but ewww) and re-run the status checks before it can merge.

For us, it's still been better to enforce that tests stay green, but we
don't have the rate of PR's that matplotlib has.

Ryan

路路路

On Fri, Nov 6, 2015 at 10:07 PM, Nathaniel Smith <njs at pobox.com> wrote:

On Fri, Nov 6, 2015 at 8:45 PM, Thomas Caswell <tcaswell at gmail.com> wrote:
> Hey all,
>
> GH now lets you protect branches which prevents people from
force-pushing or
> deleting those branches;
> https://github.com/matplotlib/matplotlib/settings/branches.
>
> I have protected our 'production' branches.
>
> There is also an option to require status checks to pass before merging
to a
> given branch, but that would break our current cherry-pick workflow and
mean
> that we could not use our judgement to over-rule travis.ci :wink:

It looks like you can set it so that the status checks are enforced on
normal users, but administrators are allowed to ignore it:
  Managing a branch protection rule - GitHub Docs

What I really want to know is whether it prevents simply pushing
directly to the branch. That would be really awesome because it would
mean you could hand out commit bits like candy without worrying that
unreviewed code could creep into the repo... but there's no indication
of this anywhere. (OTOH if direct pushes *are* still allowed, then
that also provides a method for circumventing the status checks when
desired -- just do a manual merge and push from the command line.)

-n

--
Nathaniel J. Smith -- http://vorpus.org
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel at python.org
Matplotlib-devel Info Page

--
Ryan May
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/matplotlib-devel/attachments/20151109/a94df35e/attachment.html&gt;

As a side note, I think you can force Travis to re run on top of the
current master by closing and reopening the issue.

路路路

On Mon, Nov 9, 2015, 10:21 Ryan May <rmay31 at gmail.com> wrote:

We use it here at work. Protected branches are just that--you CANNOT push
a commit (i.e. a hash) until it has passed the checks. One note, though, is
that the commit must have passed the checks running on top of *current*
master. That means if master changes (i.e. you merge another PR), you have
to update the PR (you can use the web button to merge master into the
branch, but ewww) and re-run the status checks before it can merge.

For us, it's still been better to enforce that tests stay green, but we
don't have the rate of PR's that matplotlib has.

Ryan

On Fri, Nov 6, 2015 at 10:07 PM, Nathaniel Smith <njs at pobox.com> wrote:

On Fri, Nov 6, 2015 at 8:45 PM, Thomas Caswell <tcaswell at gmail.com> >> wrote:
> Hey all,
>
> GH now lets you protect branches which prevents people from
force-pushing or
> deleting those branches;
> https://github.com/matplotlib/matplotlib/settings/branches.
>
> I have protected our 'production' branches.
>
> There is also an option to require status checks to pass before merging
to a
> given branch, but that would break our current cherry-pick workflow and
mean
> that we could not use our judgement to over-rule travis.ci :wink:

It looks like you can set it so that the status checks are enforced on
normal users, but administrators are allowed to ignore it:
  Managing a branch protection rule - GitHub Docs

What I really want to know is whether it prevents simply pushing
directly to the branch. That would be really awesome because it would
mean you could hand out commit bits like candy without worrying that
unreviewed code could creep into the repo... but there's no indication
of this anywhere. (OTOH if direct pushes *are* still allowed, then
that also provides a method for circumventing the status checks when
desired -- just do a manual merge and push from the command line.)

-n

--
Nathaniel J. Smith -- http://vorpus.org

_______________________________________________

Matplotlib-devel mailing list
Matplotlib-devel at python.org
Matplotlib-devel Info Page

--
Ryan May

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/matplotlib-devel/attachments/20151109/5f6da719/attachment.html&gt;

Also, if you go into the Travis page showing the build, there is a
"redo" circle with circular arrow that triggers a re-run. If that
suffices, it is nicer than adding the clutter of close/reopen, which
will trigger two emails to each follower.

路路路

On 2015/11/09 7:11 AM, Thomas Caswell wrote:

As a side note, I think you can force Travis to re run on top of the
current master by closing and reopening the issue.

A simple rebase doesn't suffice?

路路路

On Mon, Nov 9, 2015 at 12:20 PM, Eric Firing <efiring at hawaii.edu> wrote:

On 2015/11/09 7:11 AM, Thomas Caswell wrote:

As a side note, I think you can force Travis to re run on top of the
current master by closing and reopening the issue.

Also, if you go into the Travis page showing the build, there is a "redo"
circle with circular arrow that triggers a re-run. If that suffices, it is
nicer than adding the clutter of close/reopen, which will trigger two
emails to each follower.

_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel at python.org
Matplotlib-devel Info Page

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/matplotlib-devel/attachments/20151109/e7b181b9/attachment.html&gt;

I expect it will, but it's a little bit more complicated than clicking a
button on a web page.

路路路

On 2015/11/09 7:23 AM, Benjamin Root wrote:

A simple rebase doesn't suffice?

I think the rerun button (sadly) reruns with master of when the pr was
created (because it is pinned to the sha of that shadow merge).

I suspect that Travis and gh need to work out how to improve the web hooks
on that.

@ben yes, but is there really such a thing as a simple rebase?

The close/open cycle can be done by anyone with a commit bit, the rebase
can only be done by the contributor. I don't think we can add that sort of
bottle neck to our work flow.

Tom

路路路

On Mon, Nov 9, 2015, 12:20 Eric Firing <efiring at hawaii.edu> wrote:

On 2015/11/09 7:11 AM, Thomas Caswell wrote:
> As a side note, I think you can force Travis to re run on top of the
> current master by closing and reopening the issue.
>

Also, if you go into the Travis page showing the build, there is a
"redo" circle with circular arrow that triggers a re-run. If that
suffices, it is nicer than adding the clutter of close/reopen, which
will trigger two emails to each follower.
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel at python.org
Matplotlib-devel Info Page

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/matplotlib-devel/attachments/20151109/349703ab/attachment-0001.html&gt;

Kinda annoying when what you really want is a single button you press
that means "re-run the tests if necessary, and if they pass, then
merge".

Homu might suffice for this:
    http://homu.io/
It's designed to provide similar functionality, but predates Github's
new protected branch stuff, so projects using Homu generally just use
social convention to enforce that no-one commits directly :-). But the
homu UX does provide that 'single button' -- you post a comment saying
"this looks good to me" in the special bot-language, and then the bot
double-checks that the tests still pass when merging into current
master, and if so then it pushes them.

And, since it uses the Travis infrastructure to re-run the tests, I
think it might well automagically work with the new protected branch
feature, since Github should be able to see that the merged branch
that homu is trying to push has had the tests run on it.

(It also provides a useful serialization point -- the way Github's
protected branches work right now, if you're trying to work with them
manually and two people are trying to get a PR merged at the same
time, then they both click the "rebuild" button, they both get a green
light, they both click merge, one of them wins, and then the other has
to go rebuild *again*. With homu, they'd each post an "I approve"
message, and then Homu would take care of the race condition by
picking one of them to test-and-merge first, and then the other.)

-n

路路路

On Mon, Nov 9, 2015 at 9:20 AM, Eric Firing <efiring at hawaii.edu> wrote:

On 2015/11/09 7:11 AM, Thomas Caswell wrote:

As a side note, I think you can force Travis to re run on top of the
current master by closing and reopening the issue.

Also, if you go into the Travis page showing the build, there is a "redo"
circle with circular arrow that triggers a re-run. If that suffices, it is
nicer than adding the clutter of close/reopen, which will trigger two emails
to each follower.

--
Nathaniel J. Smith -- http://vorpus.org

(BTW, experimenting with homu is super easy: you go to
Strict Mode for your Continuous Testing | bors-ng and type in the name of your repo to register. This
by itself doesn't do much -- the only thing the bot does spontaneously
is post a comment on old PRs when they become unmergeable due to
conflicts, which is kinda nice but whatever [1]. But then if you want
to use the actual functionality, you can now type "@homu: r+" in a PR
comment, and the bot will wake up and run the tests and merge if they
pass. At least if the person making the comment is on @homu's list of
trusted reviewers.)

-n

[1] Example: MAINT: struct assignment "by field position", multi-field indices return views by ahaldane 路 Pull Request #6053 路 numpy/numpy 路 GitHub

路路路

On Mon, Nov 9, 2015 at 9:33 AM, Nathaniel Smith <njs at pobox.com> wrote:

On Mon, Nov 9, 2015 at 9:20 AM, Eric Firing <efiring at hawaii.edu> wrote:

On 2015/11/09 7:11 AM, Thomas Caswell wrote:

As a side note, I think you can force Travis to re run on top of the
current master by closing and reopening the issue.

Also, if you go into the Travis page showing the build, there is a "redo"
circle with circular arrow that triggers a re-run. If that suffices, it is
nicer than adding the clutter of close/reopen, which will trigger two emails
to each follower.

Kinda annoying when what you really want is a single button you press
that means "re-run the tests if necessary, and if they pass, then
merge".

Homu might suffice for this:
    http://homu.io/
It's designed to provide similar functionality, but predates Github's
new protected branch stuff, so projects using Homu generally just use
social convention to enforce that no-one commits directly :-). But the
homu UX does provide that 'single button' -- you post a comment saying
"this looks good to me" in the special bot-language, and then the bot
double-checks that the tests still pass when merging into current
master, and if so then it pushes them.

And, since it uses the Travis infrastructure to re-run the tests, I
think it might well automagically work with the new protected branch
feature, since Github should be able to see that the merged branch
that homu is trying to push has had the tests run on it.

(It also provides a useful serialization point -- the way Github's
protected branches work right now, if you're trying to work with them
manually and two people are trying to get a PR merged at the same
time, then they both click the "rebuild" button, they both get a green
light, they both click merge, one of them wins, and then the other has
to go rebuild *again*. With homu, they'd each post an "I approve"
message, and then Homu would take care of the race condition by
picking one of them to test-and-merge first, and then the other.)

-n

--
Nathaniel J. Smith -- http://vorpus.org

--
Nathaniel J. Smith -- http://vorpus.org

While we are in the Travis-bashing mode (sort of), is there a way to get
Travis to send a message to the creator of the PR that the tests failed?

路路路

On Mon, Nov 9, 2015 at 12:40 PM, Nathaniel Smith <njs at pobox.com> wrote:

(BTW, experimenting with homu is super easy: you go to
Strict Mode for your Continuous Testing | bors-ng and type in the name of your repo to register. This
by itself doesn't do much -- the only thing the bot does spontaneously
is post a comment on old PRs when they become unmergeable due to
conflicts, which is kinda nice but whatever [1]. But then if you want
to use the actual functionality, you can now type "@homu: r+" in a PR
comment, and the bot will wake up and run the tests and merge if they
pass. At least if the person making the comment is on @homu's list of
trusted reviewers.)

-n

[1] Example:
MAINT: struct assignment "by field position", multi-field indices return views by ahaldane 路 Pull Request #6053 路 numpy/numpy 路 GitHub

On Mon, Nov 9, 2015 at 9:33 AM, Nathaniel Smith <njs at pobox.com> wrote:
> On Mon, Nov 9, 2015 at 9:20 AM, Eric Firing <efiring at hawaii.edu> wrote:
>> On 2015/11/09 7:11 AM, Thomas Caswell wrote:
>>>
>>> As a side note, I think you can force Travis to re run on top of the
>>> current master by closing and reopening the issue.
>>>
>>
>> Also, if you go into the Travis page showing the build, there is a
"redo"
>> circle with circular arrow that triggers a re-run. If that suffices,
it is
>> nicer than adding the clutter of close/reopen, which will trigger two
emails
>> to each follower.
>
> Kinda annoying when what you really want is a single button you press
> that means "re-run the tests if necessary, and if they pass, then
> merge".
>
> Homu might suffice for this:
> http://homu.io/
> It's designed to provide similar functionality, but predates Github's
> new protected branch stuff, so projects using Homu generally just use
> social convention to enforce that no-one commits directly :-). But the
> homu UX does provide that 'single button' -- you post a comment saying
> "this looks good to me" in the special bot-language, and then the bot
> double-checks that the tests still pass when merging into current
> master, and if so then it pushes them.
>
> And, since it uses the Travis infrastructure to re-run the tests, I
> think it might well automagically work with the new protected branch
> feature, since Github should be able to see that the merged branch
> that homu is trying to push has had the tests run on it.
>
> (It also provides a useful serialization point -- the way Github's
> protected branches work right now, if you're trying to work with them
> manually and two people are trying to get a PR merged at the same
> time, then they both click the "rebuild" button, they both get a green
> light, they both click merge, one of them wins, and then the other has
> to go rebuild *again*. With homu, they'd each post an "I approve"
> message, and then Homu would take care of the race condition by
> picking one of them to test-and-merge first, and then the other.)
>
> -n
>
> --
> Nathaniel J. Smith -- http://vorpus.org

--
Nathaniel J. Smith -- http://vorpus.org
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel at python.org
Matplotlib-devel Info Page

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/matplotlib-devel/attachments/20151109/2d182cb0/attachment-0001.html&gt;