5 ms·

Facebook/Meta does not have owners, so any two people in the company can land a change.

benreesman 4y ago
And one engineer can (or could in 2018) put the unix name of the person sitting next to them in the "Reviewed By:" field and force push something, but it isn't/wasn't The Done Thing outside of the most extreme 3am SEV-0 scenarios. Any two people are in a less-extreme but similar boat: if someone puts up a bad diff and someone without a stake in the code accepts it, they had better hope to hell nothing goes wrong. SEV review is a remarkably enlightened process for what it is, but you do not want to sit there explaining to extreme-seniority people why you YOLO'd something into Presto without buy in from a Presto hacker with your manager sitting behind you already thinking about how much this is going to get harped on in calibration. The `OWNERS` file at FB is in `hg log`, but it's there.
- zeroonetwothree 4y ago
  I believe you cannot force push anymore.
  - danbrooks 4y ago
    Agree. Wasn’t aware of force push (2021-2022).
    - benreesman 4y ago
      That is probably for the best. In the bad/fun old days of extremely small on-call loop, a less clear/organized escalation chain, and fairly "roll your own" per-team tooling, once in a blue moon a whole business unit would be down because 10k machines were crash-looping and you just needed `if (NULL == ptr) return;` on like one line to end the carnage. There are exceptions to every rule I suppose. But in 7 years at FB I think I force slammed someone else as reviewer with merely their verbal approval in the middle of the night 2 or 3 times, and never after like, 2015.
  - DannyBee 4y ago
    One can imagine that auditors were not particularly enthused about this :)
    - benreesman 4y ago
      Oh anything on user data or money or illegal content shit or anything like that is a whole different ballgame. They mail you the shit off your desk for playing games with any of that stuff, and it’s thoroughly, thoroughly checked.
- eru 4y ago
  If I remember right, Google also had a mechanism for pushing something through outside of normal review. But it was something more disciplined than just faking a review by putting your co-workers name in a reviewed-by field. Because it was more disciplined, those commits could be automatically marked for later review.
avalys 4y ago
For a long time, it was worse than that - once a change was "accepted", you could amend the commit with changes to literally any other file in the entire monorepo, and land it without any further review. They finally added a "final review" step to ensure that someone eventually takes a look at these changes after the initial accept, but that still occurs several days after the commit.
- toast0 4y ago
  Can you still add Reviewed-by: yourself in the commit message to skip review? That was my favorite one.

And one engineer can (or could in 2018) put the unix name of the person sitting next to them in the "Reviewed By:" field and force push something, but it isn't/wasn't The Done Thing outside of the most extreme 3am SEV-0 scenarios. Any two people are in a less-extreme but similar boat: if someone puts up a bad diff and someone without a stake in the code accepts it, they had better hope to hell nothing goes wrong. SEV review is a remarkably enlightened process for what it is, but you do not want to sit there explaining to extreme-seniority people why you YOLO'd something into Presto without buy in from a Presto hacker with your manager sitting behind you already thinking about how much this is going to get harped on in calibration. The `OWNERS` file at FB is in `hg log`, but it's there.

zeroonetwothree 4y ago
I believe you cannot force push anymore.
- danbrooks 4y ago
  Agree. Wasn’t aware of force push (2021-2022).
  - benreesman 4y ago
    That is probably for the best. In the bad/fun old days of extremely small on-call loop, a less clear/organized escalation chain, and fairly "roll your own" per-team tooling, once in a blue moon a whole business unit would be down because 10k machines were crash-looping and you just needed `if (NULL == ptr) return;` on like one line to end the carnage. There are exceptions to every rule I suppose. But in 7 years at FB I think I force slammed someone else as reviewer with merely their verbal approval in the middle of the night 2 or 3 times, and never after like, 2015.
- DannyBee 4y ago
  One can imagine that auditors were not particularly enthused about this :)
  - benreesman 4y ago
    Oh anything on user data or money or illegal content shit or anything like that is a whole different ballgame. They mail you the shit off your desk for playing games with any of that stuff, and it’s thoroughly, thoroughly checked.
eru 4y ago
If I remember right, Google also had a mechanism for pushing something through outside of normal review. But it was something more disciplined than just faking a review by putting your co-workers name in a reviewed-by field. Because it was more disciplined, those commits could be automatically marked for later review.

avalys 4y ago

For a long time, it was worse than that - once a change was "accepted", you could amend the commit with changes to literally any other file in the entire monorepo, and land it without any further review. They finally added a "final review" step to ensure that someone eventually takes a look at these changes after the initial accept, but that still occurs several days after the commit.

toast0 4y ago
Can you still add Reviewed-by: yourself in the commit message to skip review? That was my favorite one.