Posted on February 10, 2023 by Runtime Verification
Posted in Verification, Smart Contracts
At Runtime Verification, we want to be as efficient as possible with our software development, but working on a team necessarily introduces inefficiency, which companies must manage. Being remote-first also presents unique challenges that teams must actively and intentionally overcome.
An example of why efficiency matters: suppose you have a six-week engagement with a team of two (12 person-weeks), and there are three separate half-day delays per week (seems small, right?). Then you have a minimum of 9 days delay (1.8 weeks, 15%). Delivery could have happened roughly a week earlier. Likely these delays even trigger other delays compounding the issue (delay in one timezone causes blocked work in another timezone?).
These guidelines are ones that RV uses to manage group coding and may help your team avoid introducing unneeded friction into the group coding process. They are guidelines, which means one should strive to follow them but understand that real life is often unpredictable. Therefore such procedures may need to be adjusted to accommodate the situation.
In the ideal development scenario, everyone would be merging at least one pull request (PR) daily for each project they are working on so everyone is working with the most up-to-date code possible. Merging a PR requires three main steps: (i) write code, (ii) pass CI, and (iii) pass review. Each part of this process takes time, and step (iii) requires synchronization with someone else. The overall process is just beginning when you are writing the code.
The best way to ensure low friction is to open small “obviously correct” PRs. In the smoothest scenario, you open a PR that is small and focused and makes an improvement. The reviewer can quickly tell it’s a good thing and approve/merge it before asking you for more feedback. In this scenario, the burden of creating “easy-to-merge” PRs is on the PR author. When the PR author does not make “easy-to-review” PRs, several things happen: (i) the change description cannot adequately describe the change set, thus we lose track of what changes are made, (ii) review quality goes down drastically, and (iii) the PR stalls out in the review for a while (meanwhile the code is changing, which leads to merge conflicts, and other team members are not getting any benefit from the PR yet).
Ideally, the developer would handle the to-do list in this order:
When following these steps, the time zones of the people you work with should be considered. If you have 2 PRs open that you need to review or that you need to fix, (a) one for someone who is working now and will go offline soon, and (b) one for someone who is not yet online anyway, prioritize working on PR (a) over PR (b) in your queue.
Make small, focused PRs that address precisely the problem that is needed.
Keep your git history clean and organized, with isolated changes introduced in each commit and well-tailored commit messages. Ideally, each commit should build on its own (compile) and, even better (though not always possible), should pass tests on its own. Keeping a clean git history makes every other step in the process half as difficult.
Do not have a mix of PR “types.” Different types of PRs are (i) bug fixes, (ii) new features, (iii) refactors, and (iv) code reorganizations. For example, you should not have a bug fix and a code reorganization in the same PR. This rule has many exceptions, and programming is not always done in a straight line. We often do encourage developers to make small cleanups and refactorings alongside their other changes, but following two rules: (i) anything sweeping or that grows in size moves to its own PR, and (ii) those commits are isolated from the rest of the changes, so it’s easy to pull them out if the PR reviewer asks for it.
When you open the PR, open it as a Draft, and ensure it gets tracked by the project management tools. To achieve this, you must assign yourself and then either:
When you open a PR, do a self-review on GitHub. Doing so will encourage you to look over the code the same way you expect someone else to. It also enables you to look over the code using a different presentation than the editor you are used to. Things you should check for:
Do not mark a PR as ready or assign reviewers until you have done the following:
Towards the end of your work day, if you have not opened any PRs, look at your local work and ask yourself if any subset of it is “obviously an improvement” or “obviously correct.” If so, consider pulling that chunk of work out to get it reviewed and merged so that everyone can work from that common base as soon as possible. Conducting this self-review will be easier if you carefully maintain your git history.
It is best not to spend much time reviewing PRs failing CI (unless you have been asked to provide early, quick feedback, which is good practice if you are new to a project). Just ask the PR author to get it to the point where it passes CI before requesting a review.
If the change is too unwieldy for you to review in one chunk, ask the author to break it into multiple PRs or reorganize the commit history so that each commit is logically reviewable on its own. If you do this, it is important to give specific instructions about how you would like it broken apart. For example, you can say, “pull the refactoring to module X out into its PR and the fix to bug Y into its own PR.” Ideally, PR authors are already doing this during self-review.
Things to check for in every PR:
If everything looks good on the PR, but you have some minor nits (variable names, small refactor, etc.), then DO NOT BLOCK MERGING IT. That means do not press the “Request Changes” button. You can either:
For each requested change, you should take one or more of the following actions depending on the scenario, and always make sure you indicate that you have seen and handled the comment in the GitHub interface:
GITHUB GOTCHA: When you are working with GitHub, if you force-push a branch (instead of merging), GitHub does not always know how to update the comments that existed before to display over the new version of the history (even if it’s the same contents). So you effectively lose all the conversation on GitHub up to that point. Better to merge into the base branch and keep the history (only fast-forward) so the GitHub conversations remain in context.
Do not allow scope creep:
Make sure CI is passing again.
Re-request review from the party that requested changes.
Note about trust: if the PR reviewer presses “Approve” but requests changes, and you do not address them, you will lose their trust.
This post does not discuss how to actually write code, just how to collaborate with others via GitHub when writing code. At RV, we try not to enforce any particular development style on people on their machines: we encourage teammates to use the IDE they want and manage their git history how they want. We only try to enforce basic sanity on GitHub (squash merges only for linear history, require branches to be up to date, etc.). One true thing is that you should know your tools no matter what technique you use to program. Take time to understand Git, IDE, testing tools, CI scripts, etc., to be an effective developer. Chances are, if you have some confusion or general pain point when doing development, the others on your team have already found a way to solve that problem! If not, then the team should discuss and address it.
A good read about developing code (short, <2 minutes) is “Simple, correct, fast: in that order” by Drew DeVault. In brief, at RV, we focus on producing code that is: “correct; (simple | fast); complete: in that order.” Note that we differ from the above article in that correctness is prioritized (we are producing software for quality assurance, after all), and performance is prioritized to be balanced with simplicity (our software is compute-intensive, so performance is a deliverable). Completeness is added (the idea of “handling all cases”). Following this philosophy when making and reviewing PRs has led to more maintainable code.
This post does not cover the use of CI (continuous integration) in aiding collaborative coding. Perhaps aiding is too weak a word here; fast, smooth, and ubiquitous CI is essential when developing in a collaborative setting. There are few worse things than pulling in someone else’s changes and suddenly having your local checkout stop working because of breakage introduced by them. At Runtime Verification, we have an extensive and thorough CI infrastructure and emphasize automating as much of the development process as possible. Our projects are complex and span dozens of repos, with internal dependency chains up to 6 or 7 repos deep in some cases. As much as possible, we test changes downstream early and push updates downstream automatically so that all repos use the most recent version of upstream dependencies. Perhaps we will write a post describing our CI setup in the future.
In conclusion, there are many ways teams might work together to improve productivity and efficiency when working remotely in a team environment. These strategies may only work sometimes, and they may only work for some teams. We hope that by sharing the approach that the developers at Runtime Verification use for code development, you might begin to implement and improve your own productive remote working environment.