Wow! This takes me back! Please check the date this post was authored, as it may no longer be relevant in a modern context.
As developers go through their first few years as professionals they fall in love with two things:
- A green field. A clean slate. A chance to do things right this time!
- Finishing. Completing your feature, shipping it to production, and moving onto some new task without the burdens of the last.
Unfortunately, software development is a process and not a destination. Often we don’t get to walk away from a codebase until we change jobs. Sometimes a challenging codebase is why we change jobs!
The old joke goes: If software engineers built bridges, we would all be dead. More likely, if software engineers built bridges they would add helipads and ever-wider lanes as soon as the first car was across. To sustainably and happily build software, you need to create and embrace a process (the simplest one appropriate) that prepares you for the eventuality of helipads and wider lanes.
Good software development is composed of endless beginnings, each of them ceaselessly preparing you for the next. I encourage you to think about this when you complete part of your project’s process. When opening a pull request, you’re handing something off for code review. When you complete a pass at wireframes for a new feature, you’re asking a developer to begin implementation or architecture design. What should you communicate to the next person responsible for this item? What role will you play in the next step?
Code Review: The Long Beginning
Submitting code for review starts a long chain of beginnings. It is easy for a developer to think of code submission as the end of their role (“works for me!”), but properly considering that moment as a beginning helps a project’s source stay maintainable and flexible.
- Does the code have tests? When a new features lands, it is often the easiest time to add tests. The author has a unique understanding of his or her solution to a problem at that point in time. Probably not in the sense of “is this the best solution”, but certainly in the sense of “what did I intend”. Capturing that understanding in tests will be harder later, and make subsequent work on that code much more difficult and likely to introduce errors.
- Is this code understandable? Does it have comments and meaningful variable names?
- Have you summarized the changes completely? Ideally commit messages should read cleanly, but even if they aren’t Torvaldsian in quality a well written summary submitted with your request can take up the slack. Proper communication about the intent of your changes ensures that knowledge is shared and that your reviewers have a well informed context.
- Have you provided the reviewers enough time to meaningfully consider these changes? This is a beginning for the code, and it shouldn’t be presumed that the changes are ready for production just because they are submitted. I call last minute pull requests “code bombs”- If only an hour or two is provided for reviewing a PR, the submitter is asking for a rubber stamp of approval. Submitters should mitigate this habit, and reviewers should have little patience with it. It creates a misplaced confidence in the code’s correctness and stability.
Code review (and by extension Github pull requests) are not the end of adding code to a project, instead they are among the more important beginnings. Exercise them as a chance for discussion, criticism, and bettering of the code submitted. Use them well, and may your fields be evergreen and bridges ever-wider.