6 ms·

Something I found surprising is that a change to the GitHub codebase will be run in canary, get deployed to production, and then merged. I would have expected t

by cytzol 6y ago

Something I found surprising is that a change to the GitHub codebase will be run in canary, get deployed to production, and then merged. I would have expected the PR to be merged first before it gets served to the public, so even if you have to `git revert` and undeploy it, you still have a record of every version that was seen by actual users, even momentarily. Does anyone know the pros and cons of GitHub's approach?

halukakin 6y ago
I think this method seems to get more popular by day. IMHO, previously master was the branch you merge before the deploy process. But today this is reversed. The main benefit is, other developers can rely on the master branch even more. They will know there will not be a revert on the master branch they just pulled one hour ago and already started coding on.
- Kwpolska 6y ago
  A `git revert` creates a new commit. To a developer, a revert commit appearing on master has the same effect as a pull request (or ten) being merged into it. If the revert affects code you’re working on, you will need to resolve conflicts, just like you would need to if a merged PR affected the same code.
  - irjustin 6y ago
    While what you're saying is true for all situations, the of the github flow is that to a developer relying on master, problematic code never made it in. Agreed that any codes can be added/removed but those are 100% valid changes in github flow.
    - lucasyvas 6y ago
      I thought this was what tests were for :)
      - cutemonster 6y ago
        Large software systems need all of tests, canaries, rollbacks.
bswinnerton 6y ago
This is known as “GitHub Flow” (https://guides.github.com/introduction/flow/ https://guides.github.com/introduction/flow/). I was pretty surprised by it when I first joined GitHub but I’ve grown to love it. It makes rolling back changes much faster than having to open up a revert branch, get it approved, and deploy it. When something goes sideways, just deploy master / main, which is meant to always be in a safe state.
- samc98 6y ago
  How does this work if two (or more) engineers touch the same file at the same time? EG: If two engineers have two different PRs open modifying `index.html`, how does that go to canary?
  - bswinnerton 6y ago
    Before every deploy your branch has master merged into it. There’s some clever work by Hubot while you’re in line to deploy to create a version of your branch that has the potential new master / main branch. If conflicts arise you fix them before it’s your turn to deploy.
    - rtpg 6y ago
      how deep does this deploy queue get? And does everything that lands on master have to first be deployed? This seems like a bit tough for getting work done if you have enough people all waiting around for this
      - jules2689 6y ago
        The deploy queue usually gets to be a couple of "trains" deep which usually includes work from 4-10 devs. This represents a couple of hours. We have had issues with it taking too long, but this work I wrote about has improved that! We continue to try to improve it.
- gowld 6y ago
  Why would you revert a branch (in source) before deploying it? You already have the old known good version in the history, so you should deploy that.
majormajor 6y ago
> I would have expected the PR to be merged first before it gets served to the public, so even if you have to `git revert` and undeploy it, you still have a record of every version that was seen by actual users, even momentarily. This sounds rather terrifying to me as well. Hopefully there's some sort of system for keeping around all those individual branches that made it out to prod, however briefly, for future debugging/auditing purposes if you ever need them. It's never fun to have to be doing "what code was running at this time" investigations, but every once in a while it's the only way to really get to the root of something.
- Existenceblinks 6y ago
  My ex-company has similar flow, but fix the terrifying part by (off-git-hosting, i.e. in-memory) merging ALL opened PRs to master and deploying straight to staging server. Any PR can be marked as excluded from a deploy. All PRs are ALWAYS based off master. tldr; master based pull requests as source of release.
cies 6y ago
We also make a release branch (releases/x.y.z), tag a release candidate in it (x.y.z-RCn), build it, deploy it, wait for a bit (canary stage?), it we did not want to rollback: then merge it into master.
trabant00 6y ago
I've worked with this approach for the past year: we have a "release" named branch, we deploy that, run all automated tests in production, wait a couple of hours for feedback including customer support, then merge it to master. This means master is a history of proven stable builds. And in an emergency you don't have to think about what to roll back to, it's by default master. It's just a convention, it does not have any real benefits or downsides. You could do the same with another branch named stable, or with tags. > a record of every version that was seen by actual users This is covered by the release branches and also by the CD pipeline.
- konschubert 6y ago
  This only works if you have less than one release per day
  - gowld 6y ago
    Why?
    - cutemonster 6y ago
      > wait a couple of hours for feedback including customer support, then merge it to master Seems one can deploy up to ... Say, 24/3? times per day? If 3 h in between (trabant00 thanks for explaining :-))

by cytzol 6y ago

halukakin 6y ago

I think this method seems to get more popular by day. IMHO, previously master was the branch you merge before the deploy process. But today this is reversed. The main benefit is, other developers can rely on the master branch even more. They will know there will not be a revert on the master branch they just pulled one hour ago and already started coding on.

Kwpolska 6y ago
A `git revert` creates a new commit. To a developer, a revert commit appearing on master has the same effect as a pull request (or ten) being merged into it. If the revert affects code you’re working on, you will need to resolve conflicts, just like you would need to if a merged PR affected the same code.
- irjustin 6y ago
  While what you're saying is true for all situations, the of the github flow is that to a developer relying on master, problematic code never made it in. Agreed that any codes can be added/removed but those are 100% valid changes in github flow.
  - lucasyvas 6y ago
    I thought this was what tests were for :)
    - cutemonster 6y ago
      Large software systems need all of tests, canaries, rollbacks.

bswinnerton 6y ago

This is known as “GitHub Flow” (https://guides.github.com/introduction/flow/ https://guides.github.com/introduction/flow/). I was pretty surprised by it when I first joined GitHub but I’ve grown to love it. It makes rolling back changes much faster than having to open up a revert branch, get it approved, and deploy it. When something goes sideways, just deploy master / main, which is meant to always be in a safe state.

samc98 6y ago
How does this work if two (or more) engineers touch the same file at the same time? EG: If two engineers have two different PRs open modifying `index.html`, how does that go to canary?
- bswinnerton 6y ago
  Before every deploy your branch has master merged into it. There’s some clever work by Hubot while you’re in line to deploy to create a version of your branch that has the potential new master / main branch. If conflicts arise you fix them before it’s your turn to deploy.
  - rtpg 6y ago
    how deep does this deploy queue get? And does everything that lands on master have to first be deployed? This seems like a bit tough for getting work done if you have enough people all waiting around for this
    - jules2689 6y ago
      The deploy queue usually gets to be a couple of "trains" deep which usually includes work from 4-10 devs. This represents a couple of hours. We have had issues with it taking too long, but this work I wrote about has improved that! We continue to try to improve it.
gowld 6y ago
Why would you revert a branch (in source) before deploying it? You already have the old known good version in the history, so you should deploy that.

majormajor 6y ago

> I would have expected the PR to be merged first before it gets served to the public, so even if you have to `git revert` and undeploy it, you still have a record of every version that was seen by actual users, even momentarily. This sounds rather terrifying to me as well. Hopefully there's some sort of system for keeping around all those individual branches that made it out to prod, however briefly, for future debugging/auditing purposes if you ever need them. It's never fun to have to be doing "what code was running at this time" investigations, but every once in a while it's the only way to really get to the root of something.

Existenceblinks 6y ago
My ex-company has similar flow, but fix the terrifying part by (off-git-hosting, i.e. in-memory) merging ALL opened PRs to master and deploying straight to staging server. Any PR can be marked as excluded from a deploy. All PRs are ALWAYS based off master. tldr; master based pull requests as source of release.

cies 6y ago

We also make a release branch (releases/x.y.z), tag a release candidate in it (x.y.z-RCn), build it, deploy it, wait for a bit (canary stage?), it we did not want to rollback: then merge it into master.

trabant00 6y ago

I've worked with this approach for the past year: we have a "release" named branch, we deploy that, run all automated tests in production, wait a couple of hours for feedback including customer support, then merge it to master. This means master is a history of proven stable builds. And in an emergency you don't have to think about what to roll back to, it's by default master. It's just a convention, it does not have any real benefits or downsides. You could do the same with another branch named stable, or with tags. > a record of every version that was seen by actual users This is covered by the release branches and also by the CD pipeline.

konschubert 6y ago
This only works if you have less than one release per day
- gowld 6y ago
  Why?
  - cutemonster 6y ago
    > wait a couple of hours for feedback including customer support, then merge it to master Seems one can deploy up to ... Say, 24/3? times per day? If 3 h in between (trabant00 thanks for explaining :-))