Clean Pull Requests

Pull requests can be a pain to review, but there are things submitters can do to make them easier to manage. I have some opinions about what makes a pull request "clean".

Describe What You Did

First off, pull requests typically have some kind of description field that can by default be left blank. The description is super important! Use the description field on the pull request to describe the theme of the PR. Obviously, it's ideal to have it linked to a ticket in your issue tracker.

I'm a fan of opportunistic refactoring, which I'd describe as cleaning up a bit when you're in the codebase doing something unrelated. This can get gnarly, though, in that when it comes time to submit a PR, your PR is intermingling different contexts in a way that is confusing for reviewers. In these cases, you can certainly split the work across two or more different PRs, but that can be a pain if you've already pushed a bunch of intermingled commits to one branch. In those cases, I think it's fine to keep everything in one PR, but it's incumbent upon the submitter to explain where the ticket ends and the "cleaning up" begins.

Every Line Matters

There's nothing worse than going to review someone's PR and it feels like someone dumped a "pile of changes" on you with little or no explanation to why they were made. I might be particularly fussy, but for my own PRs, I won't submit them for review until I've looked at every single line of code that I changed, and ensured that I could explain why I made it if asked. And it's not because I expect to be asked about every line I changed--in fact that would be kind of annoying, but because I often discover something that I didn't mean to change, or I had an idea for a quick refactoring like a variable rename to make the code clearer, but I forgot to do it.

One of my pet peeves as a reviewer is when a PR comes in that touches several files, but some of the files are only changed by a trivial whitespace change--like someone had been working in a file and then backed their changes out manually, resulting in maybe just an extra line break left in the file. And it's not so much that now the change list of files in the PR includes an extra file I have to look at, but because it makes me nervous that the submitter didn't pay close attention to what they had changed in the codebase before submitting the PR. It makes me wonder what other changes they included that they didn't intend to introduce into the production codebase.

It's so easy to leave in fragments of various code experiments from when you were in the phase of figuring out an approach, and then not realizing that those fragments are about to escape your feature branch and enter the official history of our production codebase.

The Clean Pull Request™

If I could sum up what I want from submitters in a Clean Pull Request™ it would be:

1. Make sure to examine each line changed in each file and ensure it's something you want to include in the production codebase.
2. Provide context for all changes included, whether that is via a linked issue/ticket that explains the task at hand, or via a written description on the PR itself that provides the context.

Keep it clean, and we'll get through code reviews faster with fewer mistakes.

Lunch & Learns

A lot of companies have a regular practice of "Lunch & Learns"—a series where engineers within the company give a presentation to their coworkers on some technical topic in which they have an interest.

I myself have given presentations at several companies where they did this, and have sat through many presentations by other engineers. Over time, I've developed a taste for what works and what doesn't.

Set the Stage

Start with the basics: Who are you? What do you work on in the company? Why is this topic important to you?

I've watched too many of these where someone starts the presentation by screen-sharing what seems like a random code file in Visual Studio (or their code editor of choice), and just starts talking in minute detail about lines of code in the file. No context at all for: what am I looking at, what is this, why is this significant, what is this application, what does this application do, why would I care, what am I supposed to do with this information, etc.

Go Beyond Generic Content

I've sat through too many "Introduction to X" presentations. They always have generic names like "React.js" or "Test-Driven Development". Now, there's nothing wrong with introductory content, but here's the thing: the audience knows how to use YouTube. If they want a generic introduction on some technical topic, there are undoubtedly free videos available that are more authoritative and higher quality.

If the content of the lunch & learn is completely generic in a way that it could be given to any group at any company without context, then there's really no good reason to be gathering a captive audience at one particular company to watch it.

Take advantage of the unique experience you have with the topic as it pertains to the projects and domain of the internal audience. Make the generic specific.

Make It Practical

What value is your audience going to get out of this presentation? Here are some lunch & learns I like to see:

• This Is How We Used X to Do Y in Application Z
• How to Replace X in Your Project with Y
• Postmortems / Lessons Learned from Project X

One of the best tech leads I've ever worked with once gave me some advice about technical presentations that I've never forgotten. He encouraged me to think about "presentation to Production". It's one thing to give an interesting presentation, or pick an interesting topic, but the next level is to think about "How are we going to put this technique/language/framework/practice into Production?"

Include the Thing

I literally wrote a blog post a couple years ago called Include the Thing about effective written communication for engineers. And it's advice that is totally applicable to presentations.

Look, no one wants to watch a presentation where someone is showing a PowerPoint deck and simply reading the slides back to you. But I think some people, especially engineers, go too far in the other direction and eschew any kind of written outline material for a lunch & learn.

It's great to "show the code", but it's next level to leave a written artifact (PowerPoint deck, Wiki page, etc.) with links to the specific internal code repositories (GitLab, Bitbucket, whatever) and individual files that you were looking at in your local code editor during your screen-share.

For lunch & learns at my current company, we do the trifecta: 1) wiki page including… 2) embedded PowerPoint deck and… 3) embedded screencast video of the presentation. Anyone coming along later who couldn't attend the lunch & learn live, or anyone who wants to refer back to the content has all the context they need.

The Yin and Yang of Technical Personalities

Categorize this post as "advice to my younger self." As a developer early in my career I would get very annoyed when coworkers and people in management at my various jobs in the software industry were not approaching the creation of software in the way that I did. Or did not have the same philosophy as mine about how this work should be done.

I'd assume that people who didn't think like me were doing things the "wrong" way, so they must be less talented, less thoughtful, less conscientious. They must not have read the books that I had read, didn't read blogs about the industry, and must not "care" like I did.

It took many experiences across different jobs of working with people who I respected, and who I could tell were very smart, but did not work the way I did, for me to finally understand that there was no "right" way to do things.

There are spectrums of personality traits in the software industry that can be summarized as a pair of opposites. Some of them might be:

Dreamers ↔ Pragmatists

Some like to think/talk about an ideal vision of a system, and some like to think/talk about the system as it is today.

Big Picture ↔ Detail-Oriented

Some take a high level view, some take a low level view. Some like to zoom out, some like to zoom in.

Move Fast & Break Things ↔ Slow & Methodical

Some value speed of change, and some value steady progress and stability.

Optimists ↔ Pessimists

Some proceed as if the best case will happen, some proceed as if the worst case will happen.

Answerers ↔ Questioners

There are so many ways to do anything in software. There are flaws in any solution, and people need to be watching for them and raising them. But at the end of the day, someone has to be willing to pick a direction and go with it.

I've tended to be toward the right side of these spectra, and would get very frustrated with people who were on the other side. What it took me years to understand was that you cannot have a successful software project where everyone is on one side or everyone is on the other side. Like so many areas of life, balance and diversity are essential for a good outcome. 

The dreamers balance the pragmatists. The visionaries need boots on the ground. The speedsters need people to check their speed. Forward progress requires leaps of optimism, even when there are a million things that could go wrong. 

To the younger me I'd like to say: different kinds of people exist for a reason, there is no "best" way to accomplish difficult goals, and you don't know everything.

Let Unsustainable Things Fail

I read a post by Max Countryman recently titled Let It Fail (Hacker News discussion), where he talks about the importance of failure in software engineering organizations. He recounts an experience he had where his boss gave him surprising advice to let a legacy system that was not getting enough attention fail visibly rather than paper over the cracks.

I've written before on this blog about how heroics cover up systemic issues. Organizations become dependent on heroics. Heroics beget more heroics. Heroics are normalized. Eventually the heroes burn out.

That one long-serving, loyal employee who had become a silo of critical knowledge up and quit. That manual, error-prone process we had been using to deploy our software for way too long finally caused an outage. We've been rushing code changes right before big releases, and one finally bit us hard.

It's important to talk openly about unsustainable engineering practices and the failure they portend. But issues of technical debt are hard to understand when placed alongside new feature development and other business priorities. The value of under-the-covers work is hard to understand. Sometimes you need to pull the covers away.

Tech's COVID Hangover

So many layoffs! I can't open Hacker News or LinkedIn without seeing constant stories about the next tech company to lay off hundreds or thousands of employees. For a couple years there, the tech industry was hotter than I had ever experienced in my career.

It never quite made sense to me, but I was riding the wave. Well, that wave has now crashed, and I'm left wondering what happened.

Ben Thompson of the Stratechery blog, a frequent guest on the front page of Hacker News, wrote recently about The Four Horsemen of the Tech Recession, and I found the analysis pretty interesting.

In particular, his discussion of the "COVID hangover" caught my attention. Essentially, demand for digital transformation in companies around the world suddenly sped up when the pandemic hit. Planned investments to modernize were all made at once, rather than the slow rollout that they envisioned before the world suddenly changed. Growth in the tech sector is now slowing as the demand was "pulled forward." Thompson explains…

When corporations the world over were forced literally overnight to transition to an entirely new way of working they needed to scale their server capabilities immediately: that was only realistically possible using cloud computing. This in turn likely accelerated investments that companies were planning on making in cloud computing at some point in the future. Now, some aspect of this investment was certainly inefficient, which aligns with both Amazon and Microsoft attributing their cloud slowdowns to companies optimizing their spend; it’s fair to wonder, though, how much of the slowdown in growth is a function of pulling forward demand.

…the COVID pull-forward was massive, but underneath the inevitable hangover there was a meaningful long-term shift to digital broadly.

The layoffs and slowdown in hiring we're seeing now after the massive boom in hiring during the pandemic is…

precisely because these [only-possible-on-the-Internet] jobs — and similarly, many of the COVID-specific workloads like work-from-home and e-commerce — were digital that it is tech that is in a mini-recession even as the so-called “real” economy is doing better than ever.

The level of growth in the tech industry that kicked off during the pandemic was not sustainable, but if it represented a pulling forward of pent-up demand, then it would make sense that there would be a slowdown in spending to follow. Hopefully after the hangover we get back to a healthy level of demand, that while not at the highest of highs, will be sustainable.

The AI Can't Destroy Anything Worth Preserving

The front page of Hacker News has been flooded lately with articles about ChatGPT, a seemingly magical AI tool that can generate realistic paragraphs of text that can seem indistinguishable from human written text.

My favorite article on the topic is ChatGPT Can't Kill Anything Worth Preserving by John Warner, a former English teacher. In that article, Warner is talking specifically about the prospect of students using AI to cheat on writing assignments, as it seems that ChatGPT can often fool the people grading them. He points out that if writing assignments are so tedious, boring, and formulaic that students aren't interested in doing the exercise themselves, and if an AI can write a passible solution, then we have to conclude that the exercise itself is absurd and not worth asking students to do. The AI is just a more convenient and efficient form of cheating than has been available to bored students for years.

Before ChatGPT was the hot topic on Hacker News, there were daily articles about GitHub Copilot and DALL-E 2. 

I've written before on this blog about the disturbing effect of automation in areas of work that one holds dear. In my post Be the Automator, I described the experience of discovering as a junior software engineer that there were tools called "code generators" that could automatically write code for you. I felt an existential threat at the time, but learned over the following years that not having to write tedious and formulaic code myself is a blessing rather than a curse.

My general feeling for many years has been that, fundamentally, the task of automation is about human liberation. It's demeaning to make a human being do something that they don't want to do if a machine can do the job just as well (or better). If the code I'm writing is so trivial that an AI assistant like Copilot can write it for me, then by all means let the machine do it! If English students are writing prose so trivial that ChatGPT can do it for them, then let the machine do it, and give them something better to do.

To paraphrase the title of Warner's article, the AI can't destroy anything worth preserving. If a machine can do it, a machine should do it. We might learn some hard lessons along the way that we weren't as creative as we thought.

Prototypes Live Forever

Code always lives longer than you want it to.

So often as developers, we're writing code under the gun. Just get it working. We have a deadline. We'll come back and fix this later.

Something I've learned through experience is that prototypes go into production. No one wants them to, but they do, 99% of the time.

That thing that you wrote that you rushed, that you're embarrassed to put out into the world—that thing will be in production, probably for a long time. Future developers, maybe long after you've moved on, will be reading it over and over, wanting to change it but being too afraid because it works, and it's been like that for a long time.

Code ossifies, the temporary becomes permanent. That next sprint where you're going to come back and clean up that code never comes. 

What does that mean for the daily practices of the software engineer? Do the little things now that can make your code better. Before the end of the sprint, before you've marked the ticket as "Ready for QA", before you've told anyone it's "done", before the pull request is merged. Give that variable a better name. Write one more (or just one) unit test that covers the basics. Split this class into two. Write a comment. Leave one more bit of feedback on that pull request. 

There are generations of developers that are counting on you. They'll be living with the shortcuts of today for years to come. I promise.