Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(todos): Adds support for configuring todo decay days by rule #1993

Merged
merged 8 commits into from
Jun 16, 2021

Conversation

scalvert
Copy link
Member

@scalvert scalvert commented Jun 8, 2021

Implements #1880 and supersedes #1964.

This change adds support for using a config file, .lint-todorc.js. This config is a superset of the old config, which was simply:

{
  "lintTodo": {
    "daysToDecay": {
      "warn": 5,
      "error": 10
    }
  }
}

The new structure adds a level of nesting, allowing you to specify which engine the config section applies to:

 {
   "lintTodo": {
     "ember-template-lint": {
       "daysToDecay": {
         "warn": 5,
         "error": 10
       },
       "daysToDecayByRule": {
         "no-bare-strings": { "warn": 10, "error": 20 }
       }
     }
   }
 }

In addition, you can now configure daysToDecay by ruleID, allowing more fine-grained configuration of decay dates. It also provides a single source of truth for configuring those dates.

As of this change, we now support

  1. Legacy configs, such as those in the first example above
  2. Configuration in package.json
  3. Configuration in .lint-todorc.js

@MelSumner
Copy link
Contributor

@scalvert confirmed that it throws the conflicting configuration file error if lint todo configurations exist in both package.json and .lint-todorc.js:

image

@scalvert
Copy link
Member Author

@MelSumner thanks for checking that. That's not a very nice user experience. We can do better by adding a util in todo-utils that checks for this case, and allows consumers to validate early, and fail with a nicer formatted message. WDYT?

@rwjblue
Copy link
Member

rwjblue commented Jun 10, 2021

lint-todo/utils#242 lets us fix that issue (still needs tweaks here once its released to actually do it though).

@scalvert
Copy link
Member Author

@MelSumner @rwjblue I updated the code to support validating the todo config beforehand, so it's cleaner output.

@scalvert scalvert requested a review from MelSumner June 10, 2021 23:22
@scalvert
Copy link
Member Author

@rwjblue any feedback on this one? It'd be nice to get it merged sooner rather than later.

Copy link
Contributor

@MelSumner MelSumner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking forward to this feature! 👍

@rwjblue rwjblue merged commit 55e92c9 into master Jun 16, 2021
@rwjblue rwjblue deleted the decay-by-rule branch June 16, 2021 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants