1. Want to run a build from a version from a few months ago, well things are fixed at the head of the main branch, but a todo in the old version are now bad and break the build. No binary searching for where a bug was introduced, or doing a patch of a previous release.
2. Github is down or you don't have internet access, now one of the key strengths of git (distributed, works offline) no longer works.
3. You have a large team, now nobody can get any work done for the length of time it takes a single person to fix the problem.
4. Your automatic cut for the weekly release runs at 3am, but a todo expired at midnight. Now the person in charge of the push to prod needs to do manual work, probably slowing down the cadence of testing the release.
5. Rust has bad compile times, this will only make it worse.
If it worked before the date, it'll keep working after unless there was some other reason for it to be added (an API shutdown deadline for example) which prevents it from working anyway.
//! # Skipping checks
//!
//! If the environment variable `TODO_OR_DIE_SKIP` is set all macros will do nothing and
//! immediately succeed. This can for example be used to skip checks locally and only perform them
//! on CI.
This makes me think a nicer (and simpler) implementation could be just special comments that have a shell command in them. Then your ci script recursive greps for commands, and runs them. That way you get enforcement in ci, not for devs, and also a super simple system with no complex compiler magic. Bonus point: it will work in any language.
I think this is a really interesting project, thought-wise. It fills in the other extreme for how important a TODO is. A code comment will certainly be forgotten, but spontaneous build failures are pretty much maximally not-ignorable.
It's almost like an assert for the environment. Maybe in the same vein they should be compiled-out for production builds? (I see in the docs they can be disabled with a flag.) But of course, if builds don't work, then people won't use it or will turn it off, like asserts.
In particular, this highlights just how much I don't think I would want to use this. Especially for environmental changes like a ticket got closed in some 3p project, or a dependency got updated. Probably the main case is the (hopefully rare) times you have to put in a dirty hack, that you _know_ will stop working at a certain date.
Otherwise, the response seems disproportionate. If it's time-intensive to fix and nothing was actually broken, failing the build is a recipe for getting bypassed. Squeaky wheel gets the grease.
Of course, proportionately motivating you to finish medium-ish term projects with questionable immediate value would solve tech debt in general :P
> In particular, this highlights just how much I don't think I would want to use this. Especially for environmental changes like a ticket got closed in some 3p project, or a dependency got updated. Probably the main case is the (hopefully rare) times you have to put in a dirty hack, that you _know_ will stop working at a certain date.
There's nothing that prevents you from commenting out the todo_or_die call that references a GitHub issue and replacing it with a todo_or_die call for next week.
That’s a really neat idea. Being able to specify certain changes as ‘not commitable’ or local only. I often find I have certain changes I always make for development and never want to commit. I always do extra git steps to avoid accidentally including them in a PR. Maybe a .env thing would help.
I can see this being useful when you're maintaining multiple versions and forward-porting patches. Some patches only make sense up to a certain version and you don't want to merge them in the next one up.
I do this with warnings, rather than build failures (for the reproducibility issues mentioned). It's especially useful if we need to work around some immediate problem, which will hopefully be fixed in later versions. I don't like depending on external state, so rather than e.g. checking if a github issue is closed, I prefer to either test that the broken functionality is still broken, or else check the version number of the relevant dependency.
Warnings are nice since we can choose how to handle them. Usually it's one of the following cases:
- A warning appears to tell us that some workaround is no longer needed, after we just bumped 'dependency-foo' to a newer version. We can delete that workaround as part of our version bump, reducing our codebase's complexity. (Deleting the workaround has a risk of causing unexpected problems, but so does the update of dependency-foo; in fact, the out-of-date workaround itself is a risk for the update, so deleting it is usually a good idea)
- A warning appears but we're rushing to hit a tight deadline. Things seem to work, so we can take a look later. (This is fine, as long as we actually do the maintenance later ;) )
- A warning appears to tell us that a dependency with known problems has been updated, so our workarounds might not be needed anymore. We try disabling the workaround, but the problem still exists in the new version; we update the check with the new version number, so the warning disappears for now but will re-appear next time that dependency changes.
- A warning appears, but we can ignore it since we're building an old version of our application (e.g. to investigate a regression).
It's a fun idea, of course not practical but a creative one. Something I've done in the past with my team was to agree to never accept a PR with TODO comments that do not have an associated JIRA ticket. So you would need to have:
// TODO (AS-1234): network errors should be handled correctly here
Where AS-1234 is the JIRA id. That way you keep track of them and they are part of your planning discussions. It's simple to do, can potentially be enforced by a linter, and help your product manager understand some of the technical debt.
If you already have a linter, I suggest going a step further and automating the JIRA issue creation when a TODO without an associated issue is found.
Nothing breaks flow quickly than getting stopped by a linter, when an autofixer could be used behind the scenes without ping ponging the developer with linting errors.
> If you already have a linter, I suggest going a step further and automating the JIRA issue creation when a TODO without an associated issue is found.
That's a huge jump in complexity and possibly introducing security issues. Not only does the CI now need to lint your code, it also has to have credentials to your planning tool, and logic for creating issues there when specific things end up in the code. All because a developer couldn't be bothered to add a ticket in the planning tool?
> Nothing breaks flow quickly than getting stopped by a linter, when an autofixer could be used behind the scenes without ping ponging the developer with linting errors.
Linting errors should show up right before commits, and (by default) prevent developers from committing if the code doesn't work (by the standards of linting, checks and tests). This is no different from static type checking then.
If you're waiting for CI runs to see what's wrong with your code, then your feedback cycle is already too slow.
> That's a huge jump in complexity and possibly introducing security issues
Possibly, I have been blessed in not having to directly deal with Jira's complexity in years. On the other hand if it has support for no read, post only ticket type permission for API tokens I see no issue.
> Linting errors should show up right before commits
That's an opinion I do not share. Don't waste my time with linters that can autofix things for you. A linter is different than a type checker. Sure some type checkers are implemented as linters, but a bunch of linters are style preference enforcers.
In short my opinion is: if your linter has a fix option, drop that in your PR CI and have it autocommit.
Oh no! A TODO is almost never written with the level of detail I'd want in a JIRA ticket. I think usually, if I'm leaving a few TODOs in a checkin, it's to highlight several locations where a single change needs to be made. Automatically creating new tickets for each call site sounds like hell.
I consider adding TODOs in the code an anti pattern. Those are usually unclear, lack ownership, and purpose of the intended changes gets obsolete. Whenever I read code with TODOs I find it that more often than not they hurt readability. Usually in reviews I ask to resolve or remove them before merge.
Open a bug based on functional (or non functional) impact. Prioritize accordingly. I think if the problem is severe it shouldn't be hidden behind a TODO. If it's not severe then likely it doesn't need attention.
Since these are mostly about things external to a project, I feel like these should be in your CI system but not in the build-time tests. Just like you would do for testing against the latest releases or latest git commits of your dependencies.
Whenever I used TODOs, they are documenting an unwanted behaviour in code that was forced by external circumstances: eg. the version of the OS doesn't support a nice feature. In this case I will add a block beginning with "// TODO(redleader55) - 2021-09-06: remove the workaround and uncomment the code below when OS supports XXXX". You can configure a linter to complain about unowned TODOs, about not having a date or an issue number in the TODO.
There are several differences between a TODO in comments and a compile time check:
1. Such a linter would work for all possible languages with minimal changes - ie. adding the pattern for a comment.
2. You can choose to run the linter only for modified lines.
3. If you need, you can run and gather all TODOs in a project or across the whole codebase from time to time.
4. You can change the requirements - ie. adding an issue/task or the owner to the text of the TODO - without having to update all the old TODOs
5. It doesn't impact the compile time, since it is a comment
It screws up reproducibility--- stuff that built in the past no longer builds when an external condition changes.
I get what this is trying to do, but there's got to be a better mechanism to let your developers know that e.g. a new feature exists upstream than suddenly breaking the build.
It shouldn't screw up reproducibility, but rather exposes what kind of unexpected state can leak in. If your build system knows what time it is and that can affect whether or not it compiles, that's a reproducibility problem.
> If your build system knows what time it is and that can affect whether or not it compiles, that's a reproducibility problem.
The very first example is of this:
// trigger a compile error if we're past a certain date
todo_or_die::after_date!(3000, 1, 1); // its the year 3000!
And the second and third examples (this is all of them) are explicitly relying on non-versioned, external state leaking in, too.
// or a GitHub issue has closed
todo_or_die::issue_closed!("rust-lang", "rust", 44265); // GATs are here!
// or the latest version of a crate matches some expression
todo_or_die::crates_io!("serde", ">1.0.9000"); // its over 9000!
But the issue is that the time your build process observes changes, not that you add a place that observes the time. If you're not fakeing time already it'll end up in all sorts of other places.
Yup, that's exactly what I was hinting at. This sort of statement makes it clear that there is an undesired connection to the system clock, but of course all those tests that use time.Now() or whatever are equally likely to become faulty.
I don't think it's accurate to say that it screws up build reproducibility. Rather, it shows that your build is already not-reproducible.
Take the time example:
if your build has access to the actual clock time, your build is unreproducible, period. For reproducibile build you need a constant, injected clock time.
So I'd rather day that if you have reproducibile builds this macros won't work at all.
I don’t think that’s true (being pedantic). If your build doesn’t depend on the value of the time in any way, then it’s reproducible (assuming it doesn’t use/doesn’t have other irreproducible inputs), even without passing in an injected time. The value of things which don’t affect the output don’t matter for reproducibility.
But it’s a one-way implication. If your build DOES depend on the time, it can be made reproducible by setting it to a constant fixed value.
You're technically correct (which is the best kind of correct!). But I would opine that, if your build system is not hermetic, i.e. you don't inject time and don't prevent access to the external world, you cannot know whether your build is reproducible or not.
Before I started using source control and issue trackers I used a macro not unlike this, but as somewhat of a roadmap or bugfix Todo.
(TODO:by-version (0 1 1) "Fix crash on leap seconds")
(TODO:by-version (1 0 0) "Implent a sane CLI")
implemented with using a macro. That was actually quite useful for all those small projects I didn't code on weekly, and scaled well for the codebases I worked on (mostly alone), which were all less than 5k loc.
I like the idea and it has a fun name albeit somewhat dramatic (vs todo-or-do-not-compile). Personally, I'd prefer to just add some defensive unit tests to a code base to check upstream functionality is behaving as expected. That said, you can't test for everything and it hasn't been the last time I've been bitten by an upstream library. This is where version pinning is helpful and where upstream vendors / developers have some useful major/minor versioning. At least with a promise (within reason) that minor version bumps are backward compatible and only major version bumps require significant downstream developer attention.
Edit: it was perhaps implicit in my comment. Normally we/I won't ship/release if unit tests fail during CI.
One really should write integration tests for dependencies. I think it is the least we can do for ourselves as well as the dependencies. Without it, the contract is basically whatever happened to work at the time you used it.
They could even be tests if your own base layer, depending on how your application is structured.
I love it. I'd most likely use them behind feature flags that I'd have enabled only during development. That way if someone is using my library/binary and they don't enable that flag, the build will never fail. But for me developing, I can get reminded of it
A coworker of mine introduced something like this (like 10 lines of python) and it's like... it's good at its job, but it's very much a "drop everything and come fix this now please" thing that blows up CI after a date.
It's pretty frustrating, to be honest, but that's more of an indicator of leaving stuff by the wayside. But you will definitely get negative vibes for introducing this strategy into a codebase, even if it's unwarranted.
I implemented something similar in ESLint. I also had it search for comments with "[Don't merge]", etc, so that I could quickly jot down in a code comment a change that needed to be made before the PR was complete. The pipeline would fail if this ESLint rule failed. It works well for me but I understand it may not work well for all.
I have a better idea: instead of adding tasks to a random line in a random file deep in the source code, add them to your ticketing system and make them part of your development cycle. They'll get triaged based on their business value instead of an arbitrary developer's discretion, making better use of resources.
I want the opposite - tickets-as-code as part of the git repo. Something like SIT[1]. Fossil has it, but today's standard DVCS is git.
There are 2 trends here:
1. Everything-as-a-code stored in the git repo in a text format close to the actual source code. If you store documention-as-a-code, diagrams-as-a-code, IaaC, tests-as-a-code, loadtesting-as-a-code, etc. - then why not issues/tickets as a code also?
2. Reduce number of tools/services/mechanisms you use in order to eliminate impedance mismatch, simplify workflows, and remove barriers to collaboration. For example if you standardize on Github, then you can use Guthub Issues, Projects, Packages, Actions, Codespaces, etc. And most developers are already having Github a/c.
This reminded me of toodles[0], a nice project you can use to visualize TODO comments as issues. I didn't have the chance to try it, but I'm looking forward it for my next project
This means that to let, e.g., testers add information to a ticket they have to do a code commit? I doubt they want to deal with that, and I don't particularly want to extend commit access to my repo to everyone who might need to interact with an issue.
Github (and plenty of other solutions) provide "edit in browser" capabilities. One could even imagine a custom web front end that enumerates the tickets, allows search, has a new/edit form that does a commit behind the scenes, etc...
Fossil SCM has all of the ticketing/bugs stored in the source control repo. Of course, in this case the repo is a SQLite database.
I still don't think I want those in the repo history, and (for a public repo) this opens up a security hole, letting arbitrary people write directly to code files. I'm not familiar at all with Fossil; perhaps its design makes this possible in a way that doesn't work so well with Git.
I use artemis to track issues/bugs in my personal projects. Each issue is an email thread in maildir format, stored in a '.issues' hidden directory in the repo. Metadata (severity, status, etc.) is kept in headers of the original message.
So we need to be a little careful here. Git has won the version control wars, and Git actually reifies a very hard-nosed and pragmatic sort of postmodernism.¹
Postmodernism gets a bad rap from modernists as being anti-Truth, and you can see this somewhat in how a Subversion diehard might view Git, “okay but what is the real truth of the code?” Git had this from kernel development, the real truth is that there are about a million different Linuxes out there due to combinatorial explosion, this one is kernel 4.15 with patches X, and Y applied, that one is 4.15 with X and Z applied...” It's not a philosophical choice but a practical reality for Linux. And so too, branch names in Git generally have process associated with them, they have to get their meaning by sociopolitical agreement among the developers and their tooling.
This conspicuous reference to tooling highlights something really important. Git’s postmodernism is constantly clashing with the modernism of Prod. We expect our websites to have one canonical authoritative Truth, at least until we start really committing to postmodernism making A/B testing etc. the norm.
You see this last point particularly in the context of Gitlab and GitHub doing CI/CD tasks.² And indeed now the general wisdom is to do trunk-based development, in other words try to ditch postmodernism at this level, thinking that way is too hard when we've got such a modernist reality in front of us, let us instead bound our postmodernism in modernism-plus-feature-toggles or or so.
Anyway, the reason I'm ranting about all of this to you is that the same modernism/postmodernism tension is present for issue tracking. Yes, logically an issue should be closed by a commit that fixes the issue. This means that an issue can be closed in production and still open in some feature branch, say. Makes perfect sense. Especially this plays well with one piece of advice I am always giving people, which is that the size of your repository, monorepo vs mini-repo, should be at the level that it makes sense to time-travel on.³
Except: we might like a very modernist view on our issues. We might want them to have a single known status, some person worked on it and finished it and now it is done. We want this issue to be in a Kanban board, say. Does a new issue get merged into all of the release candidates? Those sorts of questions. So again, trunk based development may be necessary to mitigate the mismatch.
[1] Postmodernism means a lot of different things to a lot of people. For a quick perspective to help this comment make sense, I take postmodernism as saying that there are blind-sages-and-the-elephant problems for any sufficiently interesting topic: there are multiple perspectives which each give you a partial truth, and only by seeing something from many perspectives do you get a well-rounded education on it. Furthermore, if people are saying “they're actually is only one correct perspective, one sage is Correct above all others, and it is This One” then that is best interpreted not as a genuine criticism of other ideas but as a power grab of sorts—the problem is sociopolitical and not technical.
[2] How do you configure those tasks? “With a file in the repository.” Oh, you mean a CI/CD branch that has one particular name and sets the rules for all branches, per modernist requirement? “No, like in the root of each branch’s folder structure. Like the prod and develop branches will both say “if my branch name is ‘prod’ then deploy me to production,” but this will only trigger for the one branch.” And if I push up a new branch called “hotfix” that says “deploy me to production if my name is ‘prod’ or ‘hotfix’?” Um, er, ah. This is a hard problem that both companies had to invent some sort of external-to-repo tooling for, because they both failed to understand the most basic thing about Git! You want a different take, look to Gerrit. ACLs are in the refs/meta/config branch—ah!
[3] That is, if you have microservices and it makes no sense to roll back one microservice 30 days without rolling back another, which will probably be using some new functionality that was added to the first—then you should version both together even if you build/release separately. (Git lets you get subtree hashes for this pretty easily.)
Thank you for writing this. It helped me relate a lot of half-thoughts around coordinated releases, and lack of connection between build artifacts and code (see: PyPI…).
There is a certain strangeness that we have a very distributed and local notion of code, but _running_ that code (even in local environments) is much harder.
Badly. Best I can tell, you can't even get the source for a certain build. Even if the owners bother to put a tag in git, there's no proof that's the code they used or even necessarily how they built it.
Everything on PyPI is just random artifacts uploaded by whoever ran a build script on their machine - it's nuts.
For awhile we did both-- we had some glue we'd written which turned TODO comments into issues. Then for things that were truly related to somewhere deep in the source code, they could be marked there for anyone seeing the code and in the issue tracker.
The biggest issue was, the magic we'd come up with to prevent spurious closing/opening of these issues was insufficient for formatting and reorganization, and it resulted in unnecessary churn in the issue tracker and for some comments on these issues to be lost. But done better I think it could still be valuable.
i've considered writing tooling like this. maybe not tying to ticketing systems, but instead like some sort of scorekeeping system for tech debt. maybe add some fun incentives like "if your patch removes more tech debt than it creates in terms of specially formatted TODOs, then you get to jump the CI line" or somesuch.
Check our Sonarqube. It has gating functionality that you can enable for TODOs and I’m pretty sure it can tally up number of todos as part of its tech debt calculator.
The ticketing system at work is complicated enough that I tend to avoid using it. Also I find that tickets get lost, sometimes for almost a year, before bubbling up in backlog grooming again.
The official GitHub extension for VS Code actually provides a way to turn TODO comments into GitHub issues with minimal manual interaction. This can help avoiding the context switch of opening your ticketing system rather than just staying in the editor.
1. Want to run a build from a version from a few months ago, well things are fixed at the head of the main branch, but a todo in the old version are now bad and break the build. No binary searching for where a bug was introduced, or doing a patch of a previous release.
2. Github is down or you don't have internet access, now one of the key strengths of git (distributed, works offline) no longer works.
3. You have a large team, now nobody can get any work done for the length of time it takes a single person to fix the problem.
4. Your automatic cut for the weekly release runs at 3am, but a todo expired at midnight. Now the person in charge of the push to prod needs to do manual work, probably slowing down the cadence of testing the release.
5. Rust has bad compile times, this will only make it worse.