idea: for expiring-todo-comments: warn when GitHub issue is closed

[mmkal]

I think this idea has been raised before, but I'm not sure it has with this specific restriction:

For a todo written like

// todo [#1234] until we refactor fooService, this hack prevents bar from bazing
delete bar.baz

Previously (I think) this idea has been rejected because hitting the GitHub API is an inappropriate thing to do from a lint rule, which is true. But there are many projects which keep a changelog.md file using a format like the one suggested by https://keepachangelog.com. If that changelog.md happens to include the words Fixes #1234 or Resolves #1234 or Closes #1234, then this plugin could assume that that issue is closed, and turn this todo into an "expired" one.

It wouldn't be 100% accurate, or work with 100% of projects, but might be better than not having it?

[sindresorhus]

I don't think it's a good idea to have this built-in. However, you could instead propose an option to the rule which could enable users to implement it themselves. Like some kind of hook.

To solve the asyncness, we could say that users would have to use https://github.com/sindresorhus/make-synchronous

[mmkal]

I like the idea of a hook - although to be clear, what I was proposing would not be accessing anything over the network, it would probably just use find-up to locate changelog.md and fs.readFileSync to get its content. No GitHub API. Anyway 👍 to the hook idea, there are likely too many changelog formats to reliably parse.

[saiichihashimoto]

It's true hitting up the Github API would be inappropriate, although this would be phenomenal if it could do that. Is there any "at your own risk" version of this? Often, it's not a specific version of a package that I'm concerned about, but whether or not a specific fix will be in the version.

[voxpelli]

I wonder if it could be possible to make a way to add third party "expiration providers" to extend this module, eg. leaving GitHub resolution out of this repository but allowing for someone else to add support for it.

That same mechanism could work for eg. Linear, Jira etc without this project itself having to deal with it.

Enabling a plugin to add support for eg: [plugin:github:#134] or top level [github:#134] and same for Linear, [plugin:linear:ENG-123] or [linear:ENG-123]

Top level addition would be nicer, prefixing with something like plugin: would stop potential future breakages with new built in conditions.

[mmkal]

I like that idea - or potentially even simpler, let users define a getExpiryMessage function which has the built-in value passed to it, then users can also override the default behaviour, tweak it to their liking, or do anything else they need.

Config could look like:

'unicorn/expiring-todo-comments': [
  'warn',
  {
    getExpiryMessage: (input, defaultExpiry) => {
      const github = input.match(/#(\d+)/)
      if (github) {
        const changelogEntry = findGithubIssueInChangelog(github[1])
        return changelogEntry ? github[1] + ' has been closed' : null
      }
      if (new Date().getDay() === 0) {
        // skip the default behaviour on Sundays
        return null
      }
      return defaultExpiry
    }
  }
]

The advantage being it's fully explicit and flexible - the above implementation will still work even if this library starts parsing [#1234] style annotations because it's explicitly handled. And it's a better UX for the end user to write [#1234] than [plugin:github:#1234].

Edit: my phone collapsed the full conversation - this is basically @sindresorhus's hook idea. Thread is old enough I forgot it had already been suggested!

[saiichihashimoto]

I like having an option for determining what the default is, ie [#1234] referring to a github issue in one repo, but to gitlab in a differently configured repo. I think one thing to consider is that we're often having these blocking comments because of capabilities of external libraries so, regardless, I'd want to point at whatever some-other-package#1234 is, whether it's github or jira. Also, if it didn't come as plugins maintained along with this repo, I can't imagine it'll get made (or maintained), so that's something to consider.

[saiichihashimoto]

Don't want to be the "ping" guy, but I'm having more and more comments that look like // TODO https://github.com/some-other/package-/issues/999 and it's almost exclusively how I would use this rule. Almost all the other conditions for this rule end up being a heuristic for closed issues (for me).

[sindresorhus]

I'm fine with adding a hook like getExpiryMessage.