Engineering Discipline

When you're first learning to code, you'll write a lot of code really quickly. Whether it's a personal blog, school assignment, or a side project; you've got total control over the whole project and you just go.

One of the first things I learned in industry is how slow professional software development is. This isn't to say that myself or my peers aren't working hard, but speed can be a massive liability when it comes to multi-million dollar projects.

I had a conversation with my manager today about engineering discipline in our team and our organization. Basically, if you rely on someone's good nature to behave properly in a codebase... you're in trouble. I know this because I am one of the worst offenders on my team. It's hard to give every PR the same scrutiny, and sometimes you just want to move quickly.

Before my code is committed it goes through a few automated checks and has to have two approvers. Any quality standards outside of that rely on me to be on my best behavior, and my reviewers to pay very close attention. Discipline is an important aspect of contributing to large scale projects, both in terms of writing code and reviewing it.

As soon as engineering discipline starts to slip: when your style becomes inconsistent, you opt for feature delivery instead of sound testing, and you allow each developer to put their own personal taste into the mix... you're at risk of a disaster.

Style Guides

It's hard to know what rules or guidelines make a codebase better. Do we enforce semicolons or remove them? Should we have trailing commas? Do we use single quotes or double quotes?

Without using an automated tool like eslint that enforces style checking on your code, you rely on reviewers to keep an eye out for the smallest things. That takes a lot of time to dutifully go through each line of code and nitpick the smallest style issues.

Even if it's part of your team's standard that you don't leave white space after a line, it's feels awkward leaving 50 comments on someone's PR with those nitpicky comments. It sucks to leave that kind of feedback, and it's even worse to get it. That's why we let the computers do it.

Code styling can be enforced programmatically, it won't capture everything but it can tackle the small stuff. Tools like linters will look through all of your source code and make sure it adheres to certain rules. If you want, you can add linting as part of your build pipelines and let it fail a build when the code is not styled properly.

Moving quickly and overlooking small details is a surefire way to create confusion in a codebase. It's important to be rigorous during the code review process, and not simply getting the minimum number of approvers to approve the PR.

If your team doesn't have a style guide checkout Google's style guide for a place to start and tailor it to best fit your team's needs. Be thoughtful when crafting it, and enforce it on all new PRs. If you're working with an established codebase, find time to apply it retroactively.

Unit Tests

Testing your code is important, but a lot of discussion around testing revolves around "code coverage". In my opinion, if you're writing good tests you'll have good coverage; but having good coverage doesn't mean you have good tests.

I'm struggling with keeping up with my unit tests for some of my projects just because of how quickly they're moving. I always preach to write your tests with your features, but obviously I'm finding it difficult to follow my own advice.

I think the answer I've seen that I wouldn't mind is: your PR cannot lower the coverage. Instead of setting and arbitrary threshold, which might generate bullshit tests that don't actually move the needle on having working software. You can set the noble goal that all new PRs must sustain or improve the code coverage.

When discussing unit tests and other testing strategies with your team, focus on what's being tested as opposed to how much of it is tested. Use tools that help enforce this as part of your version control & build pipelines.  

Contribution Standards & Expectations

When a repository spans across multiple teams it's important to define ownership and the standards for contributions. In GitHub you can use PR templates which give repository owners the ability to layout the fields required in a PR for review. If you look at any pull requests on a large open source project on GitHub today, you'll see these in action.

Usually there's some core team that has the responsibility of reviewing PRs and leaving feedback. Having shared ownership over different pieces of the code (be it modules or sub-repos) is very difficult, and you don't want just anyone to be able to approve PRs. That would be disastrous.

Automating PR approval is very hard. You can automate a lot of things like style enforcement, test coverage, and build output... but you can't always capture the intent of the code or making sure it captures the use case.

That's why it's even more important to enforce this rigidity and discipline when building projects across multiple teams. You're not sacrificing feature delivery by spending more time enforcing these standards, you're mitigating risk of future failure.

If your team owns a project that has many contributors, consider looking into tooling to automate as much of the PR process as possible. Enforce branch naming conventions, styling, and testing so that reviewers don't have to sweat the small stuff.