Sometimes the work on a piece of code is sufficient for a proximate goal, although it does not yet reach the âidealâ target. We are OK with merging intermediate implementations, as long as the following holds:
The implementation so far follows our engineering standards (see the rest of this wiki).
It is properly exercised in tests.
The commit messages contain release notes for the user-visible changes.
Unimplemented cases are âloudâ: they return an âunimplemented errorâ (
unimplemented.New(â¦)) to the user, so it is clear that there is work left to do.
The follow-up work has been clearly identified as technical debt (see below).
Alternate question: How does one decide whether to open a GitHub issue or leave a TODO in the code?
We have adopted the following general principles:
When possible, create BOTH an issue and a TODO. We use this approach mainly when we discover an importantÂ issue in the process of writing a PR, but it's too large or difficult to fix in the current PR.
Open an issue explaining the problem.
Reference the issue number from a TODO comment in a relevant part of the code. For example:Â https://github.com/cockroachdb/cockroach/blob/d52f071bb5277920b593711fc810fc092a2a2f24/pkg/sql/opt/exec/execbuilder/mutation.go#L155 .
If the issue number is also included in an âunimplemented errorâ or a user-visible string, also label the issue with label X-anchored-telemetry.
If it's not clear exactly where in the code the problem is or where a fix should be implemented, then opening an issue without a corresponding TODO is fine. It's also perfectly fine to just open an issue even if you know where the fix should go but you don't have an in-progress PR open for that part of the code. In that case, you can still reference line numbers or a GitHub link in the issue description.
If, in the process of writing a PR, you have an idea for something that might improve the existing code but it's not very important or urgent, or you donât know whether anyone has been interested in that improvement before, then it is sometimes ok to just leave a TODO without opening an issue.
This can be dangerous, though, since it's easy to forget aboutÂ TODOs. To counteract this, it is also useful to set up a process in your team to grep for TODOs at the end of each release cycle to make sure we did not miss something important.
We sometimes also leave TODOs without a corresponding issue number if fixing it would require more than a single issue's worth of work.
For example, in this comment there is a mention that we should "take latency into account". Making the optimizer aware of latency is a big task that is on our product roadmap but may not have a corresponding GitHub issue.
TODOs can also be helpful to explain a hacky or sub-par implementation so that a future reader of the code can improve it or at least understand why the original implementer made the choice that they did.