As posted on Twitter and elsewhere, here’s a fun little CSS brainteaser for you. No images. No SVGs. No box shadow!
Think you can reproduce this UI and interaction? Let me know how you did it.
As posted on Twitter and elsewhere, here’s a fun little CSS brainteaser for you. No images. No SVGs. No box shadow!
Think you can reproduce this UI and interaction? Let me know how you did it.
We started Radical in 2020 in the middle of a pandemic. Our mission is building absolutely killer real-time and asynchronous collaboration features that work with any existing SaaS tools.
In other words…
This idea sounds simple enough, but when you dig into what’s required to deliver this, you start to see a lot of unique constraints starting to emerge, which lead us to some very specific technical choices.
TL;DR: If you just want to know what tech stack we’re running here’s a quick run down. For more detail, see below. We’ve chosen TypeScript for both server and client; modern React with Hooks for the client UIs; GraphQL for the client-server interaction; Apollo on the client and server using WebSockets; React-JSS for managing styles; Webpack + Babel for transpilation.
We’re building tools that enhance tools. That in itself creates some super interesting constraints for how we have structure our tech stack. For instance, we can’t expect to have control of the page our code runs in. Our implementation has to play very nicely with our neighbours.
We have a heavy focus on both real-time and asynchronous features. On one extreme end, we want to be able to reflect up-to-the-second information like whether or not your teammate is typing a message. On the other end, we want to be able to reflect an entire conversation history in a tool quickly. This means we have to support short-term storage features, a lot like how WhatsApp works, and we have to support long-term storage features like complete-history retrieval, like Slack.
TypeScript is a mixed blessing everywhere you take it. It means that writing the code in the first place will be slower and more annoying. Learning all the finicky interactions between React and TypeScript is definitely a learning curve. Want to create an input event listener outside of your JSX markup? Cool, just learn the magical incantation
e: React.ChangeEvent<HTMLInputElement>. Dead simple?! Right!!? Well, yes, once you’ve scratched your head and read docs for a while. Still, once you’ve paid the bill to create the TypeScript code correctly in the first place, it becomes indispensable for maintaining your codebase. I’ve now bounced between TypeScript and non-TypeScript codebases a few times and I miss TypeScript annotation every time.
The downside of the React + TypeScript combo is that you’ve pretty much guaranteed your compilation is going to feel sluggish. If you work locally on a reasonably modern laptop, it’s tolerable. Still, even after some optimisations, we’re seeing warm-recompile times in the low single-digits seconds. That’s just slow enough to feel sluggish. No whizbang tech is free. Come on Deno!
Choosing TypeScript has huge implications for the rest of the stack, so I’ve listed it first. If you start off with TypeScript, you have to think about the TypeScript implications of every next technical choice. DefinitelyTyped will save you some of the time, but some projects are not TypeScript friendly and you’ll be incentivised not to choose them.
It may seem strange to include VS Code in the Tech Stack discussion, but the raw truth is that if you’re using TypeScript, you’re using VS Code. I grew up in the generation of developers who love to hate Microsoft. But I have to admit VS Code is the best IDE I’ve ever used. Ugh. I hate saying that. The ultra rich support for TypeScript is a killer feature. I still work in Vim/iTerm most of the time, but when I’m hacking React, I’m in VS Code (with Vim bindings!).
React was the easiest of the choices for this project. Especially since Hooks have come out, React is just the clear, clear winner over everything else out there right now. In my last role, I ported an existing PolymerJS app into modern React and generally had an excellent experience with React. The tooling support is excellent. The React core team are beasts with incredible velocity. Couldn’t be happier with this choice.
Runners up are thing like AngularJS or VueJS, but honestly, it’s not a remotely close race.
An oldie but a goodie. I’ve encountered the Metro bundler more recently in interacting with some ReactNative/Expo code and I’m curious how it compares performance-wise with Webpack. Still, I wanted something I knew and something that is reliable. The Webpack plugin ecosystem is extremely hit-or-miss, but the core tech is solid.
Still, when it slows down (which it will with all these bells and whistles and spinning rims), there is a huge body of well-documented Webpack optimisations to employ to reclaim that performance. I’m happy with Webpack for now, but I’d happily jump on something faster and simpler.
Another subtle tradeoff is that we’ve started from scratch. The upside of this choice is that we’re running the best-of-breed everything. Our codebase doesn’t suffer from any legacy ailments or poor technical choices (at least not that we know about yet!). No tech debt to pay off. Clean, linear commit history thanks to the Stacked Diff Workflow. The downside is that we’ve had to fit all the pieces together ourselves. We didn’t start with any boilerplate project helper or create-react-app. So that cost us some time. Still, this tradeoff seems easily worth it.
Like what you’re reading? Come join us! We’re currently hiring for a small number of great people to join our merry band. We’re going to change the way people work. Help us get there. Email me — email@example.com.
Recently, I’ve had the extremely mixed blessing of working with AWS and Elastic Beanstalk to deploy an app. This app has continuous delivery going using the Code* Suite. While not wonderful, CodePipeline/CodeBuild are at least straightforward.
Enter Elastic Beanstalk.
I’ve been attempting to use the CLI to trigger Elastic Beanstalk version updates. This would seem quite straight forward:
So, to update an Application, you just need to create a new Application Version and then update its Environment with a new Version Label. Bish bash bosh. In fact, that’s exactly what the
eb deploy command does (https://github.com/aws/aws-elastic-beanstalk-cli/blob/master/ebcli/operations/deployops.py#L23).
My problems started here. All of the above worked fine. However, if the app was already deployed, it would (and as of the most recent updateof this post — will) not redeploy the app. In the UI, the manifests as “degraded” state where the “Running version” and the “deployed version” are not the same. Annoyingly, Elastic Beanstalk will want to revert to the previously successful version and it will tell you that its most recently deployed version is unexpected. I mean, if the revert worked, okay fair enough, but since the revert is failing, that just leaves things in an even more confusing state.
Naturally, what I try to do is redeploy the correct version manually. That’s where the wheels really come off. Manually redeploying either in the console or via the CLI or via continuous delivery results in an unsuccessful command execution and the entire environment becomes unrecoverable.
So, what on earth is going wrong here? Well, in the AWS Console, I don’t get much:
ERROR During an aborted deployment, some instances may have deployed the new application version. To ensure all instances are running the same version, re-deploy the appropriate application version. ERROR Failed to deploy application. ERROR Unsuccessful command execution on instance id(s) 'i-XXXXXXXXXXXXXXXXX'. Aborting the operation. INFO Command execution completed on all instances. Summary: [Successful: 0, Failed: 1]. ERROR [Instance: i-XXXXXXXXXXXXXXXXX] Command failed on instance. An unexpected error has occurred [ErrorCode: 0000000001].
Not much help there.
Out of curiousity, I SSH’d into one of the EC2 instances.
sudo docker ps, I could see there were no containers running. So, then I turned to /var/log/ to see if I could find anything. I did indeed.
Digging into /var/log/eb-engine.log on the EC2 instance, I can see that Beanstalk is trying to restart nginx, but nginx is putting up a fight:
2020/04/15 00:34:55.708346 [INFO] Executing instruction: register Nginx process 2020/04/15 00:34:55.708425 [INFO] Register process nginx 2020/04/15 00:34:55.708516 [INFO] Running command /bin/sh -c systemctl show -p PartOf nginx.service 2020/04/15 00:34:55.714774 [WARN] Warning: process nginx is already registered... Deregistering the process ... 2020/04/15 00:34:55.714869 [INFO] Running command /bin/sh -c systemctl show -p PartOf nginx.service 2020/04/15 00:34:55.722615 [INFO] Running command /bin/sh -c systemctl is-active nginx.service 2020/04/15 00:34:55.729960 [INFO] Running command /bin/sh -c systemctl show -p PartOf nginx.service 2020/04/15 00:34:55.737441 [INFO] Running command /bin/sh -c systemctl stop nginx.service 2020/04/15 00:34:56.066516 [ERROR] Job for nginx.service canceled. 2020/04/15 00:34:56.066664 [ERROR] stopProcess Failure: stopping process nginx failed: Command /bin/sh -c systemctl stop nginx.service failed with error exit status 1. Stderr:Job for nginx.service canceled. 2020/04/15 00:34:56.066717 [ERROR] deregisterProcess Failure: process nginx failed to stop: stopProcess Failure: stopping process nginx failed: Command /bin/sh -c systemctl stop nginx.service failed with error exit status 1. Stderr:Job for nginx.service canceled. 2020/04/15 00:34:56.066810 [ERROR] An error occurred during execution of command [app-deploy] - [register Nginx process]. Stop running the command. Error: register process nginx failed with error deregisterProcess Failure: process nginx failed to stop: stopProcess Failure: stopping process nginx failed: Command /bin/sh -c systemctl stop nginx.service failed with error exit status 1. Stderr:Job for nginx.service canceled.
What the logs above show is that the Elastic Beanstalk engine itself is trying to stop and restart
nginx on the EC2 instance, but those commands are failing for reasons I don’t yet understand. Because these permissions are failing, the deployment process halts and the instance gets stuck in a down state.
sudo systemctl stop nginx.service, I can get the instance back and re-deploy successfully, but so far this has a 100% failure rate for all new deployments. Weird.
To get more clarity (though I have literally zero idea what the heck this means), I tried running the same command without
sudo. I get an… interesting… error:
[ec2-user@ip-XXXXXXXXXXX log]$ systemctl stop nginx.service Failed to stop nginx.service: The name org.freedesktop.PolicyKit1 was not provided by any .service files See system logs and 'systemctl status nginx.service' for details.
Wat. But the same command with
sudo (as you might expect) works a treat.
sudo make me a sandwich to the rescue.
[ec2-user@ip-XXXXXXXXXXX log]$ sudo systemctl stop nginx.service [ec2-user@ip-XXXXXXXXXXX log]$
Ho hum. I’m still chasing the root cause. Maybe this is the “unreliable deployment” cited as an issue here: https://medium.com/@acamp/elastic-beanstalk-advantages-and-drawbacks-be814615af01.
Update: For anyone working against the same issue, I managed to get around the problem by switching to immutable deployments. While *significantly* slower, immutable deployments have the advantage of always deploying to a clean EC2 instance. Because they always spin up a new instance, they don’t suffer from the restart failures described above.
Update: This post received quite a lot of healthy discussion on Hacker News. You can follow that conversation here: https://news.ycombinator.com/item?id=18119570.
People who have worked with Phabricator using a ‘stacked diff’ workflow generally love it and seek it wherever they next go. People who have never used it and only use Pull Requests with GitHub/GitLabs generally don’t understand what the fuss is all about. How can code review be *sooo* much better using some obscure tool with hilarious documentation? Well, hold on to your butts, because I’m about to break it down. This post is going to focus solely on the engineering workflow for committing code and getting reviews. I’ll probably do a second post about the details of code review between the two tools.
Before I dig deeply, let me just say I’ve created and merged hundreds of Pull Requests and landed thousands of Diffs. I know both of these workflows in and out. I’m not just some ex-Facebook, Phabricator fan boy pining for the ‘good old days.’ I’ve worked on engineering teams using CVS (oh yes), SVN, Git, and Mercurial. GitLabs, GitHub, Gerrit, and Phabricator. I’ll happily acknowledge that you can get a lot of good work done using any of these. Now, if you want to talk about how to get the most work done — the most productivity per engineer — that’s where I have a strong opinion informed by lots of experience.
Many folks reading this post won’t actually have a clue what “stacked diffs” are all about anyway. This is understandable. Feature branches and Pull Requests (PRs) are fairly ubiquitous and (sort of) well-understood. For the uninitiated, I’ll outline how it works.
In PR-based development, you start by branching master then add one or more commits which you submit as a ‘Pull Request’ in the Github UI. A Pull Request is (or at least should be) an atomic unit of code for review. When someone requests changes, you do this by adding additional commits to the pull request until the sum of these changes satisfies the reviewers demands.
The really important thing about this is that the state of your local repository is dictated by the review process. If you want to have your code reviewed, you first have to branch master, then commit to that branch, then push it remotely, then create a PR. If you get review feedback, you have to commit more code onto the same branch and a) push it to create a longer, less coherent commit history or b) merge your local commits and force push to the branch. This also means that you can’t have a local checkout of the repository that looks different from the remote. This is a really, really important point that I’ll come back to again and again.
The basic idea of stacked diffs is that you have a local checkout of the repository which you can mangle to your heart’s content. The only thing that the world needs to care about is what you want to push out for review. This means you decide what view of your local checkout the reviewers see. You present something that can be ‘landed’ on top of master. It may be helpful to skip down to the Case Studies section below to get a more intuitive feel about how this works.
The typical workflow is to work right on top of master, committing to master as you go. For each of the commits, you then use the Phabricator command line tool to create a ‘Diff’ which is the Phabricator equivalent of a Pull Request. Unlike Pull Requests, Diffs are usually based on exactly one commit and instead of pushing updates as additional commits, you update the single commit in place and then tell Phabricator to update the remote view. When a Diff gets reviewed and approved, you can “land” it onto remote master. Your local copy and master don’t have to be in perfect sync in order to do this. You can think of this as the remote master cherry-picking the specific commit from your git history.
That’s right, I said it. You can probably commit everything to master. Sound terrifying? It’s mostly… well… just, not a problem at all. It’s fine like 93% of the time. In fact, this approach gives you the ability to do things that branches alone just can’t (more on that below). The anxiety many engineers feel about committing ahead of master is a lot like the fear that if you fly at lightspeed, you’ll crash into a star. Popularly held, theoretically true, and practically completely wrong.
In practice, engineers tend to work on problems whose chunks don’t easily divide into units of code review that make sense as a branch-per-unit-of-review. In fact, most engineers don’t know exactly how their work decomposes when they start working on a problem. Maybe they could commit everything to master. Maybe they need a branch per commit. Maybe it’s somewhere in between. If the rules for how to get code reviewed and how to commit code are defined for you ahead of time, you don’t get to choose, which in many cases means a net loss in productivity.
The big “aha!” idea here is that units of code review are individual commits and that those commits can stack arbitrarily, because they’re all on one branch. You can have 17 local commits all stacked ahead of master and life is peachy. Each one of them can have a proper, unique commit message (i.e. title, description, test plan, etc.). Each of them can be a unit out for code review. Most importantly, each one of them can have a single thesis. This matters *so* much more than most engineering teams realize.
“But that’s marmot floofing crazy!” I hear you say at your computer, reading this months after the blog post was written. Is it? Is it, really?! You may be surprised to learn that many engineers, who make a fantastic amount of money from some of the best companies in the world, commit directly to master all of the time, unless they have a reason not to.
To enable this, the mental model is different. A unit of code review is a single commit, not a branch. The heuristic for whether or not to branch is this: ‘Am I going to generate many units of code review for this overall change?’ If the answer is yes, you might create a branch to house the many units of code review the overall change will require. In this model, a branch is just a utility for organising many units of code review, not something forced on you *as* the mechanism of code review.
If you adopt this approach, you can use master as much as you want. You can branch when/if you want. You, the engineer, decide when/if to branch and how much to branch.
In this model, every commit must pass lint. It must pass unit tests. It must build. Every commit should have a test plan. A description. A meaningful title. Every. Single. Commit. This level of discipline means the code quality bar is fundamentally higher than the Pull Request world (especially if you rely on Squash Merge to save you). Because every commit builds, you can bisect. You can expect that reading pure
git log is actually meaningful. In fact, in this model every single commit is like the top commit from a Pull Request. Every commit has a link to the code review that allowed the commit to land. You can see who wrote it and who reviewed it at a glance.
For clarity, let me describe the extreme case where you only commit to master. I’ll outline things that are simpler because of this. I’m starting in order of least-important to most-important just to build the drama.
With Pull Requests, if you want to catch up your local branch to master, you have to do the following:
That doesn’t sound so bad, but what about when you have a branch off a branch off of master? Then you have to repeat the last two steps for each branch, every time.
By contrast, if you only worked from master, you only have to do a git pull --rebase and you get to skip the cascading rebases, every time. You get to do just the work that you care about. All the branch jumping falls away without any cost. Might seem minor, but if you do the math on how often you have to do this, it adds up.
In the Pull Request world, this might mean I switch branches a dozen times per day. In most cases, that’s not really necessary because many of these changes would never conflict. Yet, most highly productive people do half a dozen or more unrelated changes in day. This means that all that time spent branching and merging is wasted because those changes would never have conflicted anyway. This is evidenced by the fact that the majority of changes can be merged from the Github Pull Request UI without any manual steps at all. If the changes would never have conflicted, why are you wasting your time branching and merging? Surely you should be able to choose when/if to branch.
One of the most time-destroying aspects of the Pull Request workflow is when you have multiple, dependent units of work. We all do this all the time. I want to achieve X, which requires doing V, W, X, and Y.
Sound far fetched? Well, just recently, I wanted to fix a user-facing feature. However, the UI code was all wrong. It needed to have a bunch of bad XHR code abstracted out first. Then, the UI code I wanted to change would be isolated enough to work on. The UI change required two server-side changes as well — one to alter the existing REST API and one to change the data representation. In order to properly test this, I’d need all three changes all together. But none of these changes required the same reviewer and they could all land independently, apart from the XHR and feature changes.
In the stacked diff world, this looks like this:
$ git log commit c1e3cc829bcf05790241b997e81e678b3b309cc8 (HEAD -> master) Author: Jackson Gabbard <firstname.lastname@example.org> Date: Sat Sep 29 16:43:22 2018 +0100 Alter API to enable my-sweet-feature-change commit 6baac280353eb3c69056d90202bebef5de963afe Author: Jackson Gabbard <email@example.com> Date: Sat Sep 29 16:44:27 2018 +0100 Alter the database schema representation to enable my-sweet-feature commit a16589b0fec54a2503c18ef6ece50f63214fa553 Author: Jackson Gabbard <firstname.lastname@example.org> Date: Sat Sep 29 16:42:28 2018 +0100 Make awesome user-facing change commit cd2e43210bb48158a1c5eddb7c178070a8572e4d Author: Jackson Gabbard <email@example.com> Date: Sat Sep 29 16:41:26 2018 +0100 Add an XHR library to abstract redundant calls commit 5c63f48334a5879fffee3a29bf12f6ecd1c6a1dc (origin/master, origin/HEAD) Author: Some Other Engineer <firstname.lastname@example.org> Date: Sat Sep 29 16:40:16 2018 +0100 Did some work on some things
The equivalent of the Git configuration of this might look like this:
$ git log commit 55c9fc3be10ebfe642b8d3ac3b30fa60a1710f0a (HEAD -> api-changes) Author: Jackson Gabbard <email@example.com> Date: Sat Sep 29 17:02:48 2018 +0100 Alter API to enable my-sweet-feature-change commit b4dd1715cb47ace52bc773312544eb5da3b08038 (data-model-change) Author: Jackson Gabbard <firstname.lastname@example.org> Date: Sat Sep 29 17:03:25 2018 +0100 Alter the database schema representation to enable my-sweet-feature commit 532e86c9042b54c881c955b549634b81af6cdd2b (my-sweet-feature) Author: Jackson Gabbard <email@example.com> Date: Sat Sep 29 17:02:02 2018 +0100 Make awesome user-facing change commit d2383f17db1692708ed854735caf72a88ee16e46 (xhr-changes) Author: Jackson Gabbard <firstname.lastname@example.org> Date: Sat Sep 29 17:01:29 2018 +0100 Add an XHR library to abstract out redundant calls commit ba28b0c843a863719d0ac489b933add61303a141 (master) Author: Some Other Engineer <email@example.com> Date: Sat Sep 29 17:00:56 2018 +0100 Did some work on some things
Realistically though, in the Pull Request world, this commonly goes one of two ways:
In either case, someone loses.
For Case #1, what happens when someone requests changes to V? Simple, right? You make those changes in branch V and push them, updating your PR. Then you switch to W and rebase. Then you switch to X and rebase. Then you switch to Y and rebase. And when you’re done, you go to the orthopedist to get a walker because you’re literally elderly now. You’ve wasted your best years rebasing branches, but hey — the commit history is clean AF.
Importantly, woe be unto you if you happened to miss a branch rebase in the middle somewhere. This also means that when it comes time to commit, you have to remember which destination branch to select in the Github UI. If you mess that up and merge from X to W after W was merged to master, you’ve got an exciting, life-shortening mess to clean up. Yay!
For Case #2, everyone else loses because people just don’t feel the same burden of quality per-commit in a PR. You don’t have a test plan for every commit. You don’t bother with good documentation on each individual commit, because you’re thinking in terms of a PR.
In this case, when different reviewers request changes to the code for theses V and W, you just slap commits Y++ and Y++++ onto the end of the Pull Request to address the feedback across all of the commits. This means that the coherence of codebase goes down over time.
You can’t intelligently squash merge the aspects of the various commits in the Pull Request that are actually related. The tool doesn’t work that way so people don’t work or think that way. I can’t tell you the number of times I’ve seen the last two or three commits to a PR titled with “Addresses feedback” or “tweaks” and nothing else. Those commits tend to be among the sloppiest and least coherent. In the context of the PR, that *seems* fine, but when you fast-forward 6 months and you’re trying to figure out why some code doesn’t do what it’s supposed to and all you have in a stack of 20 commits from a seemingly unrepated PR and the git blame shows that the offending line comes from a commit titled “nits” and nothing else and no other context, life is just harder.
If you happen to be one of the rare engineers who is so productive that you work on multiple, distinct problems at the same time — you still probably want a branch per-thing, even in the stacked diff workflow. This likely means that you create a branch per-thing (i.e. per distinct problem), but that you put out multiple units of code for review on each branch.
For the mortals amongst us, let’s imagine the case where an amazing engineer is working on three different hard problems at once. This engineer is working on three different strands of work, each of which require many commits and review by many different people. This person might generate conflicts between their branches, but they also are clever enough and productive enough to manage that. Let’s assume that each of this person’s branches includes an average of 5 or more units of code review in solving each of the 3 distinct problems.
In the Pull Request model, this means that person will have to create 3 branches off of master and then 5 branches-of-branches. Alternatively, this person will create 3 Pull Requests, each of which is stacked 5 commits deep with this that only go together because of a very high level problem, not because it actually makes sense for code review. Those 5 commits may not require the same reviewer. Yet, the pull request model is going to put the onus on a single reviewer, because that’s how the tool works.
The Stacked Diff model allows that amazing engineer to choose how/if to branch any commit. That person can decide if their work requires 3 branches and 15 units of code review or if their work requires 15 branches and 15 units of code review or something different.
This is more important than many people realize. Engineering managers know that allowing their most productive people to be as productive as possible amounts to big chunks of the team’s total output. Why on earth would you saddle your most productive engineers with a process that eats away at their productivity?
Every single commit that hits a codebase means more shit to trawl through trying to fix a production bug while your system is melting. Every merge commit. Every junk mid-PR commit that still doesn’t build but kinda gets your change closer to working. Every time you smashed two or three extra things into the PR because it was too much bother to create a separate PR. These things add up. These things make a codebase harder to wrangle, month after month, engineer after engineer.
How do you git bisect a codebase where every 6th commit doesn’t build because it was jammed into the middle of a Pull Request?
How much harder is it to audit a codebase where many times the blame is some massive merge commit?
How much more work is it to figure out what a commit actually does by reading the code because the blame commit message was “fixes bugs” and the pull request was 12 commits back?
The answer is *a lot harder*. Specifically because Pull Requests set you up for way more, way lower quality commits. It’s just the default mode of the workflow. That is what happens in practice, in codebases all over the world, every day. I’ve seen it in five different companies now on two continents in massively different technical domains.
You can make the argument that none of this is the fault of Pull Requests. Hi, thanks for your input. You’re technically correct. To you, I’d like to offer the Tale of the Tree Icon. When Facebook re-launched Facebook Groups in 2011, I was the engineer who implemented the New User Experience. I worked directly with the designer who implemented the Group Icons, which show up in the left navigation of the site. Weeks after launch, we noticed that almost all the groups had their icon set to… a tree. It was a gorgeous icon designed by the truly exceptional Soleio Cuervo. But… a tree? Why?
Because it was the first thing in the list.
People choose whatever is easiest. Defaults matter. So much. Even us demi-god-like Engineers are subject to the trappings of default behaviour. Which is why Pull Requests are terrible for code quality. The easiest behaviour is shoehorning in a bunch of shit under one PR because it’s just so much work to get code out for review.
This is where Stacked Diffs win out, no question. It’s not even close. The default behaviour is to be able to create a unit of code review for any change, no matter how minor. This means that you can get the dozens of uninteresting changes that come along with any significant work approved effortlessly. The changes that are actually controversial can be easily separated from the hum-drum, iterative code that we all write every day. Pull Requests encourage exactly the opposite — pounding in all of the changes into one high-level thesis and leaving the actual commit history a shambles.
The fundamental shift that the Stacked Diff workflow enables is moving from the idea that every change is a branch of off master and to a world where your work is actually a queue of changes ahead of master. If you’re a productive engineer, you’ll pretty much always have five or more changes out for review. They’ll get reviewed in some order and commited in some order. With Pull Requests, the work queue is hidden behind the cruft of juggling branches and trying to treat each change like a clean room separated from your other work. With Stacked Diffs, the queue is obvious — it’s a stack of commits ahead of master. You put new work on the end of the queue. Work that is ready to land gets bumped to the front of the queue and landed onto master. It’s a much, much simpler mental model than a tangle of dependent branches and much more flexible than moving every change into the clean room of a new branch.
(For the pedantic few out there, yes, I just said stacked diffs are like a queue. Yeah… I didn’t name the workflow. Don’t hurl the rotten tomatoes at me.)
By now, you’re probably sick of this theoretical/rhetorical discussion of what good engineering looks like. Let’s switch gears and talk about this in practical, day to day terms.
In this case study, we take a look at Suhair. Suhair is a really productive coder. Suhair produces 10 or more high quality commits every day.
Suhair starts the day fixing a bug. Creates a branch, makes changes. Commits them. Suhair then pushes to the remote branch. Then navigates away from the terminal to the Github UI to create a pull request.
Next, Suhair switches back to master, pulls, and creates a new branch to work on a new feature. Commits code. This code is completely unrelated to the bug fix. In fact, they would never generate merge conflicts. Still, Suhair sticks to branches. Works on the feature. Gets it to a good RFC state. Suhair pushes the changes. In Github, Suhair creates a pull request.
Next Suhair starts working on another feature improvement. Switches to master. Pulls. Branches. But… uh oh. This change depends on his bug fix from earlier? What to do? He goes to the bug fix PR, sees if there are any comments. One person left some passing comments, but the person Suhair needs to review it hasn’t commented.
So Suhair decides it’s too much work to create a branch off his bug fix branch and decides to do something else in the interim. Suhair pings the needed person, begging for code review, interrupting their flow and then starts working on something else.
Suhair pulls master in the morning to get the latest changes. Makes the first bug fix, commits it, creates a Diff to be reviewed, entirely from the command line. Suhair then works on the unrelated feature. Commits. Creates a Diff from the command line. Then starts working on the bug-fix-dependent feature improvement. Because Suhair never left master, the bug fix is still in the stack. So, Suhair can proceed with the feature improvement uninterrupted. So, Suhair does the work. Commits it. Creates a Diff for review.
By now, the person who should have reviewed the initial bug fix actually got around to it. They give Suhair some feedback which Suhair incorporates via interactive rebase. Suhair rebases the changes on top of the updated bug fix, which generates a small merge conflict with the feature improvement, which Suhair fixes. Then Suhair lands the change via interactive rebase. On the next git pull –rebase against remote master, the local commit disappears because remote master already has an identical change, and Suhair’s queue of commits ahead of master decreases by one.
As a bonus for Suhair today, the same reviewer who approved the bug fix is also the reviewer needed for his feature improvement. That person approved his tweaks right after they reviewed his bug fix. So, Suhair rebases those changes to be at the top of the commit stack, then lands them. Suhair never switches branches. At the end of the day, only the feature work is left in his local repo, everything else is landed on top of master.
The next day, Suhair comes in, runs git pull –rebase and starts working without any branch juggling.
Charlie is a productive, energetic, somewhat amoral hacker who just wants to get work done as fast as possible. Charlie knows the product better than any one, but doesn’t really care about code quality. Charlie is best paired with a senior tech lead (or two) who can rein in the chaos a bit.
Charlie starts the day by branching master and spamming the branch with five commits that are only vaguely related. Charlie’s commits are big, chunky commits that don’t make a lot of sense. They tend to be a bunch of things all crammed together. Reviewers of Charlie’s code always know they’ll have a lot of work ahead of them to make sense of the tangle of ideas. Because of this, they tend to put off reviewing. Today, a senior tech lead takes 45 minutes to read through all these changes, giving detailed feedback and explaining how to improve the various strands of the change. Charlie commits more changes onto the PR, addressing feedback and also makes random “fixes” along the way. In the end, the PR is probably okay, but it’s certainly not coherent and may the Mighty Lobster on High protect those who have to make sense of the code in the coming months.
During this laborious back-and-forth, Charlie’s best option is to keep piling things on this PR because all the related changes are in it. The tech lead doesn’t have a reasonable alternative to offer Charlie.
Charlie blasts out five commits and five Diffs back to back. Each one addresses something specific. Each one goes to a different reviewer because Charlie happens to be making a sweeping change to the codebase. Charlie knows how it all fits together and the tech leads can make sure that the individual changes aren’t going to ruin everything.
Because the changes are smaller and more coherent, they get much better review. A tech lead points out that one of the changes is clearly two separate theses that happen to touch the same set of files. This tech lead reviewing the code pushes back on Charlie. The tech lead points out that these should actually be two separate commits. Unfazed, Charlie abandons the Diff. Using interactive rebase to rewind history to that troublesome commit, Charlie uses git reset to uncommit the single commit that has two theses.
At this point, Charlie’s local master is two commits ahead of remote master and has a bag of uncommitted changes that Charlie is currently hacking on. There are two more changes that are in the future, waiting to be added back to the local commit history by Git when Charlie is done rebasing interactively.
So, Charlie uses git add -p to separate out one change from the other and creates two new commits and two new diffs for them separately. They each get a title, a description, and a test plan. Charlie then runs git rebase –continue to bring fast-forward time and bring back the later changes. Now, Charlie’s local master is six commits ahead of remote master. There are six Diffs out for review. Charlie never switched branches.
Yang is a great engineer working in a fun part of the infrastructure. Unfortunately, Yang has a bad neighbour. This other engineer constantly lands the team in trouble. Today, Yang has found that the build is broken due to yet another incomprehensible change. The neighbour has a “fix” out for a review, but no one trusts it and several knowledgeable people are picking through the code in a very contentious code review. Yang just wants to get work done, but can’t because the bug is blocking everything.
Yang will checkout the remote branch with the “fix”. Next, Yang will branch off of that branch in order to get a sort-of-working codebase. Yang gets to work. Midday, the bad neighbour pushes a big update to the “fix”. Yang has to switch to that branch, pull, then switch back to the branch Yang has been working on, rebase, and then push the branch for review. Yang then switches gears to refactor a class nearby in the codebase. So, Yang has to go back to the bug “fix” branch, branch off it, start the refactor, and push the commit remotely for review. The next day, Yang wants to merge the changes, but the “fix” has changed and needs rebasing again. Yang switches to the bug fix branch, pulls. Switches to the first branch, rebases. Pushes. Switches to GitHub to do the merge, carefully selecting to merge onto master rather than the bug fix branch. Then Yang goes back to the terminal, switches to the second feature, rebases, pushes, goes to GitHub, selecrs to merge to master, and merges. Then, Yang applies for AARP, because Yang is now in geriatric care.
Yang sees that the Diff for the “fix” is out for review. Yang uses the Phabricator command line tool to patch that commit on top of master. This means that it’s not a branch. It’s just a throwaway local commit. Yang then starts working on the first change. Yang submits a Diff for review from the command line. Later, the “fix” has changed, so Yang drops the patch of the old version from the Git history and patches in the updated one via interactive rebase. Yang then starts working on the second change, submits a Diff for review. The next day, Yang is ready to land both changes. First, Yang dumps the previous patch of the fix and repatches the update to make sure everything works. Then, Yang uses the command line via interactive rebase to land both of the changes without ever switching branches or leaving the terminal. Later, the fix lands, so Yang does a got pull –rebase and the local patch falls off because it’s already in master. Then Yang goes to skydiving because Yang is still young and vital.
As you can see from the Case Studies, you can definitely get good work done no matter what tool chain you use. I think it’s also quite clear that Stacked Diffs make life easier and work faster. Many engineers reading this will say the cost of switching is too high. This is expectable. It’s a thing called the Sunk Cost Fallacy. Everyone prefers the thing they feel they have invested in, even if there is an alternative that is provably more valuable. The stacked diff workflow is a clearly higher-throughput workflow that gives significantly more power to engineers to develop code as they see fit.
Inside Facebook, engineering used the branch-oriented workflow for years. They eventually replaced it with the stacked diff workflow because it made engineers more productive in very concrete terms. It also encourages good engineering practices in a way exactly opposite to branching and Pull Requests.
Something I haven’t touched on at all is the actual work of reviewing code. As it turns out, Phabricator also happens to offer better code review tools, but I’ll save that for another post.
Update: After a few pieces of earnest feedback that I’m actually taking away from people’s experiences, I decided to alter the details of this post to exclude direct mention of the movie. Given the strong negatives of the overall experience, I feel like what movie it is doesn’t really matter. Still, doesn’t do to diminish someone else’s time. Fair warning: This post still contains details from the event that are pertinent for giving a coherent review. Some people may feel these details are spoilers. I’m including them not to be a jerk, but to be illustrative.
Wanted to offer some feedback about Secret Cinema from Friday, March 21st. This is my second Secret Cinema experience and I was really looking forward to it. My first was the Casablanca showing about a year ago.
Aspects of this Secret Cinema event were very good. I’ll just list them for brevity’s sake:
That said, the were some extremely negative aspects of the experience. To be honest, the negatives of the experience so far outweighed the positives, I don’t think I’m likely to participate in Secret Cinema again. Especially considering that some of the negatives were consistent from my first time at Secret Cinema last year.
When we arrived, we were greeted by someone who told us to go “over there” where a group of people were gathered, so that we could get our hands stamped. We were immediately asked to show our tickets by a woman patrolling the line. I had the email on my phone, so I started loading it. The person asking me was very abrupt with me. She then moved on to the next people in line. As the line moved towards the front, I was asked my last name, and had my hand stamped without ever seeing the tickets. Which begs the question — why was I questioned about my tickets if all that was needed was my last name? Also, while waiting in this line, a man dressed as a police officer swore at the people in line in front of us and told them to speak when spoken to. Okay, if this were a prison movie or if we were in character as suspects for something (or kidnapped people perhaps) that makes sense. However, if his character is an officer on the street and we’re upstanding citizens, what sort of behaviour is that? Just felt oddly mean and degrading for no clear reason.
When we entered the Chinese Laundry, we were greeted by a character who was to shepherd us into the underworld. This character was openly antagonistic. Beyond being a bad actor whose awkward Boston-meets-Chicago-meets-London gangster accent was painful, he seemed to be just indulging in the opportunity to swear at us without making it feel like a legitimate experience at all. The mismatch between what would have made sense and what he was doing was so conspicuous that it took away from the experience. If he had chosen to say nothing at all, the effect would have been more resounding and interesting.
Once we were inside, I was greeted by a tall guy in a dark suit and hat. He bumped into me from behind. I turned to face him and apologised, unclear at that point that this was his act. Face to face, he leaned in close and blew e-cigarette smoke in my face. I could smell his not-clean breathe and feel the moistness of it on my lips. Gross. He then said, “You want to make a hundred bucks?” Let’s take a step back and think about real life. Let’s imagine that we’re in the real world. I’m in a club. Some guy walks up to me, runs into me, blows smoke in my face, then asks me if I want to do work for him. How would I feel? I would feel like punching the guy. I would settle for getting away from him as quickly as possible because he’s a really unpleasant guy to be around. So, given that this is ostensibly play (though there was nothing playful about him blowing smoke in my face), I just said “No. Not remotely.” He looked at me disgusted, told me “You’re not going to have any fun around here,” and walked away.
Ironically, he was pretty much right. What he didn’t understand is that I already wasn’t having any fun. No aspect of the experience so far put me in a state of freedom to play. Nothing so far felt like there were safe bounds between reality and play and that we were all in this together. No, in fact the experience felt much more power imbalanced in favour of those who knew what was going on than those who didn’t.
Secret Cinema feels more like the Stanford Prison Experiment than a playful immersion in the world of the movie.
Sadly, the list of bad interactions keeps going.
At one point, we were all ushered into a real court room to witness a fake trial. I was grabbed aside and drawn into a room with a guy who told us to render “Not Guilty” no matter what in exchange for $500 bucks. This seemed like the moment to be playful for me. Getting to participate in an intense jury debate sounds fun. Sadly that wasn’t the way this worked. I was to do as I was told here. Being told what to do and when to do it is not fun. Then, in the court room one actor posing as a bailiff told us to stand in one place. Then another came along, swore at us, and told us to go stand somewhere else, asking who told us to stand there in the first place. Hilariously, it was his fellow actor. Being treated like dogs expected to roll over again and again is not fun.
At another point, we tried to go into a club but we were stopped at the door by an actress. She told us it was members only and that we couldn’t just walk in “unless we had something for her.” So, playing along, we offered her as much money as we had. Yet, it wasn’t good enough. She kept up the act of surly door minder to the point that my date and I just felt truly unwelcome and walked away. It wasn’t mean-with-a-wink, it was just mean.
Later, I was standing in one of the rooms and a guy walked in, pointed harshly at me, said, “You — come with me,” and stormed out. Not a chance in hell. By this point, I was so fed up with the experience that I would rather stand off with the guy right then and there than engage in any more “play.”
I can keep going, but you get the point. I’ll leave out the other court room bailiff who stopped us to extract a “jury fee” from us and all the other small interactions that were antagonistic by default. Why couldn’t actors just have interesting personalities? Why was aggressive, exploitive behaviour the default interaction with every person? Sure, the movie they showed was a gritty movie, but that doesn’t mean that every person you meet should treat you badly as the first note of the interaction. Also, the fact that everyone in the movie is a grifter is ostensibly a secret, so we should have had no idea why the hell everyone was being so abusive. The whole thing feels like an indulgent experience for the “actors” and an abusive experience for the attendees.
The most absurd reality of this is that I’m an extremely playful person. I love fancy dress. I love acting a fool and not taking myself seriously. I do a lot of public speaking and feel very comfortable in a semi-structured environment that is semi-contrived and semi-real. I was once kicked out of a public park with some friends for acting out Hamlet on the amphitheatre stage after the park had closed. For god’s sake, I went to Burning Man last year and had an amazing time. I also got really into this night. I rented an authentic 20s suit and followed all the procedures ahead of time. So did my date. I wanted to come and have an exciting and playful night. If someone like me can’t get into Secret Cinema, who the hell is it for? Who would have a good time?
Back at the start of the night, after getting our hands stamped, we walked to where we had to give up our phones for the evening. Part of my work is building systems that operate at all hours of the day on complex infrastructure. Sometimes systems break and I have to act quickly to figure out what’s going wrong. It makes me very uncomfortable giving up my phone because it’s my notification mechanism when things go wrong. Also, I use my phone as part of two-factor authentication to lots of systems. If they lose my phone, it would be more than a day of work to get things back in order, starting with buying a new phone and a trip to my service provider to get a replacement SIM. Not to mention the trouble it would be trying to get them to cough up the money for the phone later. Not remotely my notion of a good time.
So, I questioned their procedure, asking what assurance I have that they were going to do this correctly and not cause me tons of grief. The response I got was a compassion-less “We haven’t lost a phone yet. This is company policy.”
Interestingly, none of the information I was given ahead of time included an explanation that this would be required. I went back through every email and every web page I was directed to. No mention at all. If they had told me ahead of time, maybe I would have changed my mind about going. Maybe that’s their reasoning behind surprising you with it when you get there.
The guy taking my phone explained that I would be given a ticket that would allow me access to my phone at any point if I needed it urgently. Unhappy but not willing to end the evening five minutes into it, I gave up my phone, took my packet and went inside.
At the end of the night, my date and I went to retrieve our phones. There was already a group of people starting to accumulate, but we were first in line. A guy still acting half-way in character took our ticket, radioed for our phones to be brought up then walked away. Minutes into this, with no more info about what was happening, one of the other people waiting had someone return with a satchel with his ticket’s number on it. The woman returning his phone peeked inside the bag and then asked him to describe his phone to her. This is their system. They decided that a number and a satchel was the correct way to do this. Interrogating people afterward is insane. If you take my phone from me and give me a reclamation means for it, I do not expect to also have to prove my knowledge of my own property. It also rang closely of being asked for my tickets at the entrance when my name was all that was required.
Being honest, I lost my temper at this. I spoke harshly to this lady. Something along the lines of “Just give him his fucking phone back.” Reasonably, he looked surprised. She looked antagonistic right back at me and told me this was necessary because they wanted to make sure people got their correct phone. I would argue that it’s complete bullshit and that they should come up with a more reliable system if they wanted to take ownership of tens of thousand of pounds of nearly identical devices. If I found any ticket lying on the ground, went to them, answered their question with “an iPhone,” I would have a super high likelihood of walking away with a phone that wasn’t mine. It’s a bad system at best. Also, when we got our phones later, they didn’t ask for a description. Arbitrary treatment where someone else has power over you and you just have to do whatever they cook up for you to do is not fun.
Around that minute, chaos ensued. A group of people down the hall had been setting up a table for returning phones to people. It was unclear who we should be talking to. It was also completely obvious that the organisers had done nothing ahead of time. They were sorting the phone satchels numerically while fifty-ish people stood and waited. This was around 5 hours into the evening. It didn’t occur to them to have this done ahead of time? That at the end of the movie people would want to leave? Or to make sure the phones stayed in order as they were collected? I cannot understand why this was an issue at all. Every coat check I’ve ever been to just gets this right by default.
The group of us waiting for our phone grew from the original ten, to fifty, to probably half of the people at the event. This is an interesting data point, because we were leaving early. As we walked out of the movie hall, multiple people told us to stay because there was a dance afterward. We didn’t exactly feel like dancing. When the credits rolled, I felt like I had put up with the fullness of the experience and, relieved, could now leave. Perhaps some of the more than a hundred people gathered in the hallway felt the same way.
So at this point we were waiting somewhere between the desk and the table (theoretically, our phones would come to the desk where the guy had radioed for them), we heard someone shouting “Over 200 on the right, under 200 on the left”. They were trying to partition a mob of more than a hundred of people *after* it had become a mob. Also at this point, despite having left early to get out before the crowd, my date and I were now swimming in a mob of people with no idea where our phones my show up — the desk where the guy had radioed or the table where the mob was looking.
We walked to the tables (where they were still shuffling phones while people waited) and asked. We were told to please wait in line. There were no lines, just two mobs of people running the length of the hallway now. I was feeling sort of apoplectic at this point so I just walked away. I would rather sit quietly somewhere than wait in an infinitely long line while they sort out something that should never have been a problem. My date was more persistent with the person holding back the mob, got a bit aggressive, and had our phones returned immediately. I feel bad that we jumped the mob. I feel like we got special privilege because we were rude, which is not something I ever want to be. This feeling is counterbalanced by the fact that we took special efforts to get ahead of exactly that situation and the incompetent handling of the situation ruined that and left us standing around with no notion of what to do until the situation was going to be a mess no matter what.
When we got back home, my date noticed that they had managed to scratch up her screen protector in the process of confiscating and returning our phones. No damage to the phone, fortunately, but it indicates that they weren’t going out of their way to take care of our phones.
In sum, Secret Cinema made for a bad evening. If I had it to do all over again, I would skip the event without batting an eyelash. I would have had more fun watching a lot of other movies alone.
This is the first post of the newest iteration of my corner of the web. Enjoy and converse. In case you’re curious, this is a WordPress blog. I’m not a WordPress fanboy, but it is the best tool for the job as far as I’m concerned. I rebuilt most of the blog twice before switching to WP. The first time, I wrote a directory parser/cacher system that would let me statically generate the blog from flat files. The second time, I wrote an even more sophisticated version that was still file-backed rather than database backed. I was doing a lot of fancy caching, minimizing, and dog fooding. Still, I wasn’t getting it done fast enough. Who cares who fast a blog loads if it isn’t done enough to launch. Exactly no one, including me. After spending probably fifteen hours on each I had the epiphany that just using WordPress will deliver significantly more bang for the time I spend. At the time of this writitng I have approximately 7 hours in the project and it’s nearly complete in terms of desired layout and features for desktops and tablets. The handheld mobile view sucks, but I haven’t spent any time optimizing for that yet. I’ll get there. Anyway, now that this bad boy is up and running, look for frequent updates with interesting things.