4 ms·

I am hesitant to write this, because in general I agree, but let's say a product manager stops a feature short of completed in some sense, even though there are

I am hesitant to write this, because in general I agree, but let's say a product manager stops a feature short of completed in some sense, even though there are some aspects which would make the feature more robust, secure or optimized, the code gets released and one would need a tombstone as a visible nuisance and indicator for these aspects, wouldn't it? I mean the best place to communicate important ideas and warnings about code, is the code itself isn't it?

Jach 7y ago
If you've let a PM stop a feature short of something you as a dev think is rather important, you've already failed to some extent as a professional. There are ways out of such a mess, but it's better to avoid it to begin with, and if it really couldn't be helped, to have a paper trail if not only for yourself then for the benefit of others. Coming to the project as a new hire, I'd find a "TODO[3 years ago]: maybe add a cache for X to speed things up on this flow" comment less illuminating than "We think this will probably start to fall over at some scale point because we're not using DB connections wisely (and maybe want a cache layer) but our PM didn't give us time [or 'we weren't able to stop the PM shipping before we had time'] to perf test and rework the flow that was patterned after all the other DB-conn-happy logic". (Not that either comments are that useful -- guess why I'm reading this part of the code?) But I'm mature enough to think, coming across such code without any of those two comments, that the latter scenario is more likely than "Argh stupid devs, stupid this-dev-in-particular who I can see with git-blame, probably never even thought of a cache! I don't even need to see if there was discussion in the review!" That sort of thought only comes with direct experience dealing with people who repeatedly show themselves as professional failures who should at least retire into management but even better should find a different career field... Comments like "NOTICE: Don't optimize this loop or you'll introduce a timing attack!" are always welcome in my book. Though if whatever you're warning about can be enforced with a unit test, that's even better insurance against someone naively or even idiotically changing it.
- ilammy 7y ago
  > TODO[3 years ago] The thing is, you might also be not reading that piece of code for the sake of performance optimization. In this case such TODO is a testimony of premature optimization being evil: like, the code has obviously been fine for at least three years without any smartypants optimization that the developer at that time envisioned to become necessary. Though yeah, it could have equally been just a 3-year-old ticket in the issue tracker. However, you won't learn about it when casually exploring the code.

Jach 7y ago

If you've let a PM stop a feature short of something you as a dev think is rather important, you've already failed to some extent as a professional. There are ways out of such a mess, but it's better to avoid it to begin with, and if it really couldn't be helped, to have a paper trail if not only for yourself then for the benefit of others. Coming to the project as a new hire, I'd find a "TODO[3 years ago]: maybe add a cache for X to speed things up on this flow" comment less illuminating than "We think this will probably start to fall over at some scale point because we're not using DB connections wisely (and maybe want a cache layer) but our PM didn't give us time [or 'we weren't able to stop the PM shipping before we had time'] to perf test and rework the flow that was patterned after all the other DB-conn-happy logic". (Not that either comments are that useful -- guess why I'm reading this part of the code?) But I'm mature enough to think, coming across such code without any of those two comments, that the latter scenario is more likely than "Argh stupid devs, stupid this-dev-in-particular who I can see with git-blame, probably never even thought of a cache! I don't even need to see if there was discussion in the review!" That sort of thought only comes with direct experience dealing with people who repeatedly show themselves as professional failures who should at least retire into management but even better should find a different career field... Comments like "NOTICE: Don't optimize this loop or you'll introduce a timing attack!" are always welcome in my book. Though if whatever you're warning about can be enforced with a unit test, that's even better insurance against someone naively or even idiotically changing it.

ilammy 7y ago
> TODO[3 years ago] The thing is, you might also be not reading that piece of code for the sake of performance optimization. In this case such TODO is a testimony of premature optimization being evil: like, the code has obviously been fine for at least three years without any smartypants optimization that the developer at that time envisioned to become necessary. Though yeah, it could have equally been just a 3-year-old ticket in the issue tracker. However, you won't learn about it when casually exploring the code.