

Protected branches and required status checks - sanjeetsuhag
https://github.com/blog/2051-protected-branches-and-required-status-checks

======
ajross
This sounds like the wrong solution to me. The problem isn't (or rather, is
rarely[1]) that you've "polluted" a public branch with bad code. Bad code gets
pushed into branches by perfectly legal commits all the time, and you fix it
via software engineering and not administrative policy.

The real problem with the accidental force push is that it's lossy. The OLD
head, to which you would hope to revert when you realize your mistake, is
suddenly invisible (and garbage-collectible!).

So I'd much prefer to see github fix this with a "hard reflog" of the branch
state on the server. Track the head of each branch at each point in "monotonic
server time" (with a tag or whatnot, or even outside the repository would be
fine) such that you can always (1) detect force push events like this and (2)
trivially revert them.

[1] Yeah, occasionally you might push your site keys, or some NDA-covered
source code. And in those situations you _want_ the reverting push to actually
"forget" the code. But that should be the rare exception and not the standard
process.

~~~
sampo
> The real problem with the accidental force push is that it's lossy. The OLD
> head, to which you would hope to revert when you realize your mistake, is
> suddenly invisible (and garbage-collectible!).

I prefer how Mercurial behaves in this matter. A push can not destroy old
heads, and the old (dangling) heads still show up in the log by default (you
can filter them out with an option, if you don't want to see them).

So if someone pushes a new head by accident, anyone can still see what is
going on, and fix the situation either by a merge, or reverting back to the
old head.

(The downside is, if everyone really knows what they are doing and never make
mistakes, git's way of deleting old heads with just a force push is easier,
than needing to specifically access the repo on a server if you want to delete
some heads.)

~~~
masklinn
Technically a push does not destroy heads in git either (that requires a gc),
but contrary to mercurial "detached heads" are not visible by default, you
have to explicitly go and hunt for them.

~~~
sampo
Yes, you are correct. The problem is not so much with git, it is with GitHub.
You cannot retrieve dangling heads from GitHub, not even by cloning. Maybe
GitHub runs garbage collection right away?

But if you just keep your repositories on your computers, then yes, you can
clone and then find the dangling commits in the clone, too, and then save
them.

~~~
masklinn
> You cannot retrieve dangling heads from GitHub, not even by cloning. Maybe
> GitHub runs garbage collection right away?

Nah, since you can undelete branches on github. It's just the way git works:
it only retrieves (clone/fetch) named heads and whatever is reachable from
there, so detached heads aren't fetchable.

------
masklinn
A good fix is to religiously use `--force-with-least` rather than `--force`.
Sadly because Git I know no way to make force-with-lease the default and make
—force less convenient (outside of a `git force` alias, which isn't going to
disable `push —force` so you'll have to train it into your muscle memory)

force-with-lease checks that the _actual_ remote head and the local one match
before pushing, which prevents overwriting changes you aren't locally aware
of, which I found fixes almost all force-push trouble.

~~~
dperfect
I think the point is to protect a branch at the origin so that the mistake of
using the wrong options doesn't cause damage. Training everyone on a team to
do the right thing is good, but everyone is prone to mistakes (myself
included, even after using git for years).

~~~
masklinn
You'll get no argument from me there.

------
tylerdiaz
I'm a little surprised to see it took this long to release what some teams
consider an essential feature. I worked at a company where force-pushing
master (by-accident) would put the master branch in lock down while someone
could fix its history, which is easily solved by this feature.

~~~
asb
Github still offer pretty weak configuration of permissions. e.g. the Dolphin
team decided against migrating to Github due to the poor flexibility in
permissions. [https://dolphin-emu.org/blog/2015/09/01/dolphin-progress-
rep...](https://dolphin-emu.org/blog/2015/09/01/dolphin-progress-report-
august-2015/) You can't have users who have the ability to triage issues
without also giving them write access to the repository.
[https://help.github.com/articles/permission-levels-for-an-
or...](https://help.github.com/articles/permission-levels-for-an-organization-
repository/)

~~~
chralieboy
We struggle with that last one all the time. Our sales team can't add issues
with tags or assign them without write access to the repo. So they just have
to dump them in issues and then engineers have to categorize them.

~~~
sytse
In GitLab there is a more granular permission model, reporters can edit the
labels but not see the code, see [https://gitlab.com/gitlab-org/gitlab-
ce/blob/master/doc/perm...](https://gitlab.com/gitlab-org/gitlab-
ce/blob/master/doc/permissions/permissions.md)

------
krishicks
This can also be done safely from the git cli with the `--force-with-lease`
option. `--force-with-lease` will only do the push if the latest ref you've
fetched from the remote matches the current ref of the remote. There's a few
options you can give it, too, so I recommend reading the docs: [https://git-
scm.com/docs/git-push](https://git-scm.com/docs/git-push)

------
killercup
Preventing force-push is only one small part of a good solution _.

_ good solution:

1\. create pull request which is automatically being tested by CI

2\. instead of hitting 'merge', tell build bot to take pull request and merge
it into temporary copy of master

3\. if tests pass on build bot, it applies pull request to master (by pushing
temporary branch)

4\. never ever push to master yourself

This is also basically what [http://homu.io/](http://homu.io/) does.

------
rajnp
Gitlab has protected branches feature for quite sometime. I was wondering when
Github is going to implement it. Finally!!!

~~~
Pyrodogg
The benefit of competition. I hope Gitlab, Github and others keep pushing each
other forward. It's better for everyone.

~~~
jobvandervoort
We'll do our best.

It's a very interesting market. I think we're (we as in: GitLab, GitHub,
Stash) all very aware of each others features and shortcomings, not in the
least because of the intense comparison that every customer does before they
decide on a purchase.

At GitLab a large part of our new features are driven by requests of customers
or shortcomings that they noticed in competitors' products. In the end, we
also believe this benefits everyone.

------
leehro
This is great, particularly the "Update Branch" feature. Prior to that, the
only way to update a branch (or a fork for that matter) was to use an external
client to merge in upstream changes and push back to GitHub.

I worked on a project with several non-developer users updating content. The
web interface works well for simple HTML/CSS changes, but if we developers
moved or renamed a file, their forks would immediately be useless. This
feature should make that a non-issue.

~~~
brandonwamboldt
I kind of wish you could merge or rebase as part of the "Update Branch"
feature. I don't allow merging master into the feature branches for any
project I manage, I insist that developers rebase their branches before they
get merged instead.

------
geofft
Does the "required status check" thing apply to `git push`, or just to the
merge button on the website?

If it applies to `git push`, how do you trigger a status check to run on your
actual merge/rebase as made in your local client, which will almost certainly
differ in SHA1 and may differ in content from the merge made in the PR?

~~~
bhuga
> Does the "required status check" thing apply to `git push`, or just to the
> merge button on the website?

It applies to all ways of updating git: push, the web UI, merging, and the
API.

> how do you trigger a status check to run on your actual merge/rebase as made
> in your local client, which will almost certainly differ in SHA1 and may
> differ in content from the merge made in the PR?

If they differ in SHA1 but not content, things will work fine. A protected
branch cannot be updated to a git tree that has not been tested.

If your PR differs in content, clicking the 'update branch' button on a PR
will make the merge commit and your PR's content the same, so a new CI run
will apply to the correct content.

~~~
geofft
Thanks! That's a very sensible model. I suppose it means that the status-check
API can't be used to check that the history of a branch is up to some
standards, like commit messages following a style or all commits in history
passing some check, but that's fine. (It is a bit at odds with the
documentation of the status-check API, which implies that it's about refs, not
trees.)

~~~
bhuga
The API was originally about SHAs, rather than refs, but for protecting
branches trees make sense. (You can retrieve statuses by ref, but are required
to set them by SHA). Since a SHA can be uniquely resolved to a tree, we're
doing this under the hood rather than make a breaking API change. It also just
makes more sense; nobody thinks in terms of trees day-to-day.

Thanks for you comments about per-commit linting. We'll give them some
thought.

------
kcthota
finally...every time there is a release, managers want to lock out the branch.

Gitlab has similar thing to prevent checkins. Glad similar feature is coming
to github as well..managers in enterprises will be happy..

------
larrik
As a mercurial user, how easy IS it to accidentally force-push in Git? You
have to really go out of your way to do it in mercurial, and I haven't found
any reasons to do so.

~~~
masklinn
> how easy IS it to accidentally force-push in Git?

It's easy to accidentally force-push _to the wrong branch_. If you're on the
wrong branch (e.g. you think you're on your personal dev branch and want to
update it after a rebase but you're actually on the mainline) `git push -f`
will ruthlessly clobber the remote.

~~~
tedunangst
This sounds like "it's easy to accidentally shoot yourself in the wrong foot",
implying that shooting yourself in the other foot is a good thing? Maybe take
a step back and ask not whether it's the right or wrong foot, but whether any
foot shooting is necessary?

~~~
masklinn
> Maybe take a step back and ask not whether it's the right or wrong foot, but
> whether any foot shooting is necessary?

Github/PR workflows often (usually?) recommend cleaning up pull requests (via
rebase -i) in the same way submissions of patchsets get reorganised and
cleaned up after comment then resubmitted (whether to a mailing list or a
patch review tool like gerrit) rather than adding new cruft on top of the
initial patchset.

That does indeed require "foot shooting" as the old branch needs to be
replaced by the new one, lest each resubmission lose all existing discussion
and comments and each tentative change ends up using half a dozen different
pull requests.

------
robotnoises
This could have saved my morning...

