Hi maintainers. π
First off, thanks for developing and maintaining this package, it's been really useful!
However, I have a minor unintuitive nitpick with how the expiring-todo-comments
rule handles partial versions.
To preface:
>
and >=
, and a "valid SemVer version" is expected after the comparison operator.However, I believe, there is a minor unintuitive case here where versions that don't specify all three positions.
When reading/writing the TODO comments and encountering or writing >4
, my brain goes
Anything within v4.x.x will pass lint and this TODO won't trigger lint until we upgrade the major version past v4.x.x
and same goes for >4.1
where it goes
Anything within v4.1.x will pass lint and this TODO won't trigger lint until we upgrade the major past v4.x.x or minor version of v4 past v4.1.x
However, what this really means is:
>4
is actually >4.0.0
due to coercion, which will get triggered by any version higher than 4.0.0
>4.1
is actually >4.1.0
due to coercion, which will get triggered by any version higher than 4.1.0
>=4
is actually >=4.0.0
due to coercion, which will get triggered by any version higher than 4.0.0
, including 4.0.0
>=4.1
is actually >=4.1.0
due to coercion, which will get triggered by any version higher than 4.1.0
, including 4.1.0
This can be avoided by suppressing my own monkey brain:
>=5
or >=4.2
>4.9999.9999
or >4.1.9999
but ... yeah, no.One might argue that even the documentation gets this wrong. It's a bit hard to assume the intent here due to missing context but I believe I can assume the context, so I am using them to support my assertion of unintuitiveness by showing the problems the examples have.
The examples that are currently here state:
// TODO (@lubien) [>0]: For now this is fine but for a stable version we must refactor.
// FIXME [>10]: This feature is deprecated and should be removed from the next major version.
First example will always trigger at any version other than 0.0.0
.
>=1
as we assume all v0.x.x
are unstable in SemVer and that the author will have multiple unstable versions like 0.0.1
and 0.0.2
and the first "stable version" is v1.0.0
or greaterThe second example will trigger on 10.0.1
and higher, while the intent seems to be to say >=11
.
9.x.x
range then the "next major version" intent fails because 10.0.0
won't trigger the rule, therefore it should be >=10
10.0.0
, then the "next major version" intent fails as it will trigger the rule even on 10.0.1
10.x.x
(excluding 10.0.0
), then it will trigger right away while we're still in any v10.x.x
range and the "next major version" intent fails.Of course, there are other examples that reason about this correctly, but these two seemingly don't, and it's understandable why.
I mean, nothing is really wrong here, it works as underlying logic would dictate. It's just a bit unintuitive, and when writing such comments, I shouldn't really assume 4
expands to a range 4.x.x
but to version 4.0.0
specifically.
Readme examples might need correcting, if my assumptions on them are correct.
This would fix the assumed unintuitiveness in this specific case, but would probably be a breaking change to the rule.
Even the support for explicitly setting X-range (1.x.x
, 1.1.x
) version seems like it would be a breaking change as they also currently get resolved in the same manner. The lack of intuitiveness here would remain unless the support for partial versions is also removed to prevent it.
I don't believe this is quite a bug or a new rule, but I believe it's closer to a bug, so I've created it as that.
Also, apologies for any premature issue notifications, opened the issue prematurely by accident. π
Pay now to fund the work behind this issue.
Get updates on progress being made.
Maintainer is rewarded once the issue is completed.
You're funding impactful open source efforts
You want to contribute to this effort
You want to get funding like this too