Protected branches and required status checks | Hacker News Reader

Protected branches and required status checks

github.com

109 pointssanjeetsuhag47 comments

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.

> 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.)

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.

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.

> 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.

> Track the head of each branch at each point in "monotonic server time

oof talk about over-engineering. What is the point? Just don't allow force pushes to protected branches is a much simpler model than. "Keep a bunch of bookkeeping meta data in the event of an unanticipated force push". All this complexity would only save you during the span of time those objects were collectible but not yet collected.

Not really following your point. What I described could be implemented by a post-update hook that just ran "git rev-parse HEAD" and stuffed the result into a database somewhere. If anything it's significantly easier to implement than an authorization model.

I'm not following where you are going. If these database records point to objects in the git database. How will you synchronize garbage collection and the state of the database?

With pointers out of the refs directory, the way you do it for everything else? Are you deliberately trying to start a fight? This isn't a complicated subject...

Not trying to start a fight, I just don't understand your solution. What does git do with orphaned entries in the refs directory when it garbage collects? Does it delete them? How would you synchronize your database pointers with garbage collection? Presumably when this happened you'd have to interrupt it or you'd have to delete those records from your database. Then they'd really be lost right?

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.

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).

You'll get no argument from me there.

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.

I don't think github was created with a shared repository model in mind. If you are forking which is the implied model this feature is relatively pointless because they offer you "pull only" from forks.

I'd love to see a source for this. I'm not trying to call bullshit on you or anything, but I can't imagine any hosted git implementation not at least accounting for branches. Forks are the "GitHub way," but branches are the "git way," no?

Sure I mean I can't give you a source per se but I think anyone that has been using github for a few years is familiar with the "forking" model( evidenced here with github giving steps for how to synchronize your forks - https://help.github.com/articles/syncing-a-fork/)

Branching is a separate concept and its certainly accounted for in the forking model. But we are talking about the process driven meta layer that github has stacked on top of git. Their original proposed workflow did not have a bunch of folks working out of the same shared repo instead(and this even extends to their enterprise model) every user that start working on a project would fork it. You'd make your changes and contribute them back to the authoritative repo as a "Pull Request". Branches model an evolutionary line of code, but a "Pull Request" models a change that you want to introduce typically into another repository. The implication here is that forks and branches are separate concepts that share some overlap. I often branch in my fork and contribute my change from my forked branch to be introduced into `master` in the authoritative repo without ever delivering the branch itself. Rather simply merging the changes into `master` as a discrete unit of work. Github's polite suggestion that you work this way is further evidenced by encouraging you to delete merged branches. I find this to be satisfactory but many people have personal hangups with deleting branches for reasons that are not entirely clear to me. This model allows me to sync my fork's version of "master" periodically with master in the repository of record(typically referred to as "upstream") and bring those changes into my branched work.

"Forks" and "Pull Requests" are not git concepts they are github concepts. Forking provided a nice little metaphor for locking down a repo because you couldn't create a disaster for anyone but yourself. Many git saavy organizations do not allow direct access to the authoritative repo instead only allowing "Pull only" access. This allows the authoritative repo to pull the changes that add value and reject those that do not without adding lots of cruft. This harkens the "Social Coding" aspect that github wanted to develop in their earlier days. With everyone contributing to everything forking left and right. Businesses wanted to piggy back of the toolchain they created but most folks weren't familiar with the idea of social coding and/or had needs that weren't addressed well by the social coding model. Github said no worries I think I can tailor this to a business. Which is why you have lots of fork based tools for private repos( organizations can take ownership of private forks, can force forks to be private, can set organizational ACLs on forks and private repos, etc, etc)

Some not so git/github saavy organizations work out of a shared repo for reasons that aren't very clear to me. Its usually a misinterpretation of who owns forks and/or the visibility of private code. I get a sense that github fought these ideas for a long time just saying "c'mon friends just use forks" and that this is their aim at a compromise.

>branches are the "git way," no?

This takes us to this. The answer is sort of. Really cloning is the git way. You clone a repo and synchronize it with other people's repos. Most organizations realize pretty quickly that some repo has to be the repository of record, but git doesn't care. To it a repo is a repo and you know what you are doing. Github just layers a little process on top of the "git way" turning a "clone" into a "fork" and add a little ceremony to the contribution process.

Wow, this is a really extensive answer, thank you. I really like the pull request model encouraged (enabled?) by GitHub -- it codifies the code review process, which is handy, especially when automated testing isn't up to par yet. That said, for dev teams, I'm not totally sold on the process.

>Some not so git/github saavy organizations work out of a shared repo for reasons that aren't very clear to me.

I've used the private GitHub services (private repos and Enterprise) with different companies for different projects, and the whole GH workflow seems to encourage something pretty clearly outside the realm of continuous integration. While the people I've worked with/for have almost always said we were working on building out the "CI/CD pipeline," the pull request workflow always ended up as a blocking function stuffed inside what would otherwise be called continuous integration. It's not continuous at all -- it places the onus of "merges" on people looking at stuff instead of trusting the tests, which means tests never get written and PRs are humongous.

Anyway, my point is that I like working out of shared repos because it makes it easier to just totally bypass the pull request workflow, though you're right, it can present the opportunity for rebase nightmare destruction derbies. For open source projects and big distributed things, pull requests are invaluable. But it seems kind of counterproductive in most of the scenarios in which I've seen it.

>and the whole GH workflow seems to encourage something pretty clearly outside the realm of continuous integration

The reason for this isn't clear to me. PRs are nothing but a `git merge` wrapped in a web ui that shows you a preview of the diff. Since those concepts are equivalent you have to then say "merging is pretty clearly outside the realm of continuous integration", but I'd say thats the concept that makes CI possible. By virtue of their equivalence PRs(if you choose to use them) can make CI possible too.

I want to be super clear about this using "Forking" and "Pull Requests" have zero limitations when it comes to CI/CD. In fact compared to working out of a shared repo there is only exactly one difference. Since your clone's master can diverge from authoritative master you have to periodically synchronize masters(thats that "sync your fork" link I put in the original post) I'll admit this is almost a justification for not using forks. Its annoying and tedious to explain to new git users.

>It's not continuous at all -- it places the onus of "merges" on people looking at stuff instead of trusting the tests

This sounds a bit off to me. Continuous integration is about getting good code into the delivery pipeline. Once that code is there you want to get it public as quick as possible. There is a relationship that describes the cost of bugs and bad design decisions as exponential given their proximity to getting into the customers hands. The tests will find regressions but won't stop a bad design or a design with a new bug, or a piece code without any tests at all. There are two things to note here catching things early is hugely cost effective and that code reviews are a vehicle for probing a completely different class of problems. The GH workflow is built around code reviews. This is because in the "Social Coding" model you want to make sure whatever rando is delivering code into your repo is respecting your style/development guide. Most organizations want this benefit as well. Do code reviews slow things down? ... yes. But I'd say thats a feature not a bug :) Are "pull requests" not "continuous". I don't understand exactly what you mean by that or what its value is. But it doesn't seem terribly useful by itself.

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... 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...

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.

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...

I always thought that this feature was left out as a differentiation for Github Enterprise, where you could do it with hooks.

But a simpler answer may be that it was just hard to implement.

GitHub Enterprise has had the ability to deny force-push to master for a little over a year now. I think it might have had the ability to totally deny force-pushes before then, too. They seem to be steadily improving the granularity of the controls.

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

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/ does.

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

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

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.

Protected branches have been a feature of Bitbucket also for a long time.

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.

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.

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?

> 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.

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.)

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.

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..

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.

same deal. If your history can't be reasonably stacked on what exists it'll stop you and give you a warning. Only by `git push -f ` can you force it to happen. With that said I work with a lot of folks that only have a passing understanding of git internals. With that in mind I'm I'll sleep better at night knowing that my precious master branch won't be subject to any late night Stack Overflow spelunking disasters.

> 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.

Yep. I force push frequently to feature branches on my remote when preparing pull requests. Being able to protect against this on the canonical branches of the main repo is a nice addition.

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?

> 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.

Someone much wider than me once told me: if I ever felt the need to use the force flag as part of a command (hg or not), odds are I'm trying to do something I shouldn't be doing. As it turns out, I haven't used that flag since, because whenever I thought I needed it, it turned out I was going about something the wrong way.

That being said, like you, I use hg, haven't used git very much.

I use git heavily and haven't ever actually had to force-push. It means that something's gone wrong, and you should really understand what.

And especially, especially if this is a team branch. At least make a copy of the old branch with a backup name, so you can undo your damages easily. It takes like 1 second.

As a Git user, it seems Very Hard to accidentally force push: you need to add a command line parameter, and I was simply taught "Don't Do That". I've certainly never done it.

e.g.:

  # yes!
  git push origin my-feature-branch
  git push origin master

  # no!
  git push --force origin master

nsfyn55 makes a good point about protection from others, though. Maintaining my own fork, and using pull requests to the central repo, seems to help avoid others clobbering master, but that seems to be more of a reflection of my organization's Git workflow.

If your team treats Git the way we used to use Mercurial, where each developer will merge their changes into master and then push it, you're going to be in a WORLD of hurt. (It's likely we were doing it wrong there, too. I didn't really grok distributed source control until we moved to Git.)

If you use the Github Flow [0], the merging is done in the UI of your Github (or Gitlab/etc) instance, rather than directly with `git`. (I believe Atlassian's stash has a similar but slightly different recommended workflow.) The keys for us are:

  - One team repo is the "official" repo. (e.g., UI/foo-widget)
  - Each dev forks the repo, and pushes their branches on that fork 
    (e.g. gknoy/foo-widget:gk-feature)
  - Pull Requests are made from the dev's repo __to the official one__ 
    (e.g. from gknoy/foo-widget's gk-feature branch 
          to UI/foo-widget's master branch)

Some teams instead opt to have each dev make branches on the same (official) repo. I prefer the extra safety net of maintaining my own fork. If I were to mistakenly commit something (or merge something, or rebase something) incorrectly, and totally fubar my repo's master branch, and then push that up to my origin before noticing it, I can recover it easily (rename branches, checkout the commit that should be the head, re-push). Worst case, I can delete my fork and re-fork it. ;) If I were to do that on a repo that I have shared access to, I'd have much more anxiety.

Incidentally, I've made exactly that mistake (merged or committed something onto my local master, and then pushed it to my origin) TWICE, and caught when others have when doing code reviews. The first time, I panicked and deleted/re-created my forked repo. The second time, I fixed it by juggling branches, and it was substantially easier. (Slower: it took me ~20 minutes I think?) In neither case did my mistakes affect anyone else, though, since the repo I messed up was my own.)

This is, of course, based heavily on Github's suggested practices, since we use Github Enterprise at work. Were I using Stash or GitLab, there would likely be some changes, but I think the team workflow really benefits when you dive all-in on Git's distributed nature.

0: https://guides.github.com/introduction/flow/

This could have saved my morning...

Reply on news.ycombinator.com