You have a choice in this moment. You can stay quiet and hope it goes away or point it out, question it, and even argue against it. But so often, engineers fold. They ignore their conscience and their gut in the interest of a steady paycheck and an easier work day. Avoid conflict at all costs, especially when that cost could be their job. “Just keep your head down and do what you’re told”, they think, while they twiddle their thumbs as bad product decisions whoosh by. Sure, they complain about it over drinks with coworkers and in one-on-ones, but they don’t say anything when it counts.

We’re better than this. As software engineers and designers, we’re in the room when decisions are shaped, and the only ones who have the power to actually execute them. It’s our responsibility not to forsake the people who trust the apps we make with our silence. To stand up and refuse to implement unethical systems and dark patterns. And even more, to educate stakeholders on the real human costs of their business decisions: the time, attention, money, and trust of their customers.

It’s harder, yes, and riskier. But they can’t build it without us. We get a say. Even if it’s not in that meeting, we can think about the goals they’re trying to accomplish and propose alternatives. We don’t have to hide in our sit-stand nap pods and eye-roll while we engineer a worse world. We can do more than write code. We can research and present better alternatives. We can write memos and make slide decks to convince them of our position. We can be activist engineers.

Even though these bad ideas may buttress the metric-of-the-week, they’re at the direct expense of consumer trust and customer satisfaction. They’re a tax on our company’s reputation. We have to push the people making the decisions to measure more than just the number they’re trying to increase. Look at reviews, net promoter score, social media mentions, and team morale. All of these trends matter to the long-term health of the company, and should be treated as such.

This requires long-term thinking and the kind of organization that’s receptive to it. In many companies, quantifiable short term gains are valued more than long-term, qualitative investment. The best companies resist this temptation to make a quick buck and build upon a lasting mission and principles. But even in companies with lofty vision statements, things can go awry. A bad quarter can send the company’s hard-won principles out the window to make room for the growth hackers.

---

In other disciplines, engineers wear an iron ring to remind them of their commitment to their profession. Though we may not be part of the Order of the Engineer, we can learn a lot from their obligation:

As an engineer, I shall participate in none but honest enterprises. When needed, my skill and knowledge shall be given without reservation for the public good. In the performance of duty, and in fidelity to my profession, I shall give the utmost.

Of course, not every idea you dislike is a bad one, so spend your reputation thoughtfully but forcefully. Make your dissent count, but don’t be a jerk.

Our job as software engineers is to build things that make the world (or a corner of it) better, things that solve problems. But that’s not our only job. It’s also to be gatekeepers: to prevent ideas that we know are harmful from being realized. What’s the worst that could happen: we get a reputation for giving a damn?

Code review, or more generally peer review, has a long record of finding defects in not just software engineering, but in science, academia, and many other industries. While some argue about the specific percentages of issues that it finds compared to an automated test suite, or others method of testing, code review is an incredibly useful tool in any software teams arsenal against bugs and poor software architecture. But only if it remains a sharp one. Laziness can creep into code review very easily if you’re not careful.

Jason Brennan has written two great pieces on the topic of diligently writing and reviewing pull requests, and I’ve used his series as an inspiration for my own system. I’ve also found it’s handy to have a short-form checklist to go over when performing code reviews to remind myself what I’m going for. Maybe it’ll help you and your team too.

For the engineer drafting the pull request

• Provide a screenshot, GIF, or video of the change if possible. People like pictures.
• Explain what changed and why.
• List step-by-step how to test the changes.
• Link to any relevant task(s) or ticket(s) in the bug tracker.
• Link to any existing documentation that could make the change easier to understand for the reviewer.
• Mark any areas that are work in progress or require follow up.
  • Note anything that is waiting on other departments or team members.
• Call out any legal, security, or privacy concerns.
• If any third-party dependencies have been added, explain what they are and why you chose them.
• Double check that the code is styled, documented, and tested to your team’s standards.
• Mention people who would be interested in the changeset:
  • Engineer(s) who wrote the old version
  • Designer(s)
  • Product manager (if they’re interested)
• Give the diff one final pass yourself before asking others to take a look. You might catch a few silly things like typos in comments.
• Annotate particularly tricky sections in the diff to make what’s going on even clearer. Maybe even turn these into code comments.
• Get a coffee and wait for the constructive criticism to roll in.

For the reviewer

I find it’s helpful to do these in order if you can.

Most importantly, remember that the engineer on the other side of the screen is a person. Try not to be curt or hurtful in your comments.

The Pull Request

Start here to build an understanding of what you’re about to be diving into.

• Let your teammate know that you’re reviewing their work however’s customary: a comment, a message, making yourself the assignee.
• Read over the pull request description and any linked documents to understand the nature of the changes.
  • Ask questions about anything you don’t understand or if there isn’t enough detail to properly review.

The Product

Does it work as you’d expect? You can write lots of pretty code that doesn’t work.

• Checkout the pull request locally.
• Confirm that the project builds and compiles.
• Look for any new warnings or errors that have been introduced.
• Run the unit / integration tests to look for any new failures.
• Follow the steps provided to test the changes or determine a testing plan for it and run through those tests, noting any issues in your comments.
  • Test things that the original engineer might not have considered like errors, failures, other inputs.
• Look for significant changes in performance.
  • Profile the changes if performance seems to have regressed.
• Run the application in another language and region to check for internationalization and localization issues.
• Confirm that the changes are fully accessible to customers with disabilities.

The Code

Alright, it’s time to start looking at the code, line by line.

• Deleted code
  • If code was deleted, was its functionality adequately replaced or is what it provided no longer required?
• Added code
  • Does the new code introduced conform to the teams’s standards for style, documentation, and testing?
  • Does new code make sense when read? Is anything too clever or inexplicable?
  • Is new code safe?
    • Does it have the potential to crash or hang?
    • Are there any obvious race conditions or concurrency concerns?
  • Is new code fast?
    • If not, can you suggest optimizations?
  • Is it well-designed and well-factored?
    • If not, try to propose alternative solutions or schedule a time to whiteboard with the engineer.
  • Is it well-named?
    • Are the names of types and functions obvious and unambiguous?
• Modified code
  • Do you understand why the modifications were made?
  • Do the modifications improve the factoring, design, or performance of the software?
  • Do the modifications change any of the fundamental assumptions that the original code made?
• Dependencies
  • Make sure to understand the implications of new dependencies on third-party code, including their licenses.
• Assets and Resources
  • Do all assets and resources exist in all the right formats and sizes?
  • Are they finalized and ready to go into production?
• Configuration changes
  • Do you understand the effects of these changes and how they can be rolled back?

---

Adding a system like this to my own pull request reviews has helped me catch a lot more, but I’m sure there are still things I’m missing. If you have ideas about how this checklist can be improved, pull requests are welcome!