Warning comments are allowed without warning

[ianwremmel]

Generally, I'm a fan of setting no-warning-comments: error since they get lost really easily and never acted upon. Historically, we've had no-warning-comments: warn in place since there are already so many in the code base. At a minimum, I'd like to up that to warn and, ideally, error.

[adamweeks]

https://eslint.org/docs/rules/no-warning-comments

I'm good with them being an error and just disabling the linting specifically for a TODO that needs to go into the project.

[ianwremmel]

yea. i'm partial to error myself. I'd also love to get this thing wired up, but last time I tried it got weird with our jenkins flow.

[bzang]

What if we only allow a select few specific comments? If it's only a select set, it maybe less likely to get lost.

I actually do find TODOs useful. Having to add a eslint-disable for every TODO seems much. This gets to be a bit much for projects that have precommit hooks that lint. IMO it's reasonable to have a TODO while I'm working on a feature. Maybe some sort of enforcement on PR merge? Is that possible?

[ianwremmel]

yea, agreed. maybe we put the general rule at warn, but then change all all our projects to maybe have a .eslinrc.js that conditionally sets it to error when CI is true?

[adamweeks]

I don't think eslint-disable-next-line is unreasonable. I put FIXME and TODO in the code to remind myself of something to do before merging.

Just checked and in all of the widgets we only have one instance of TODO.

[bzang]

I don't use TODO because it errors/warns me not to... also I'm OCD and the squiggly lines drive me insane. I like @ianwremmel's idea of doing warn and then switching to error so that it doesn't make it into master.

[ianwremmel]

@bzang. yea. most of the comments in here are why i've had them set to warn. It's enough to remind me to do stuff, but I can still commit while working on a PR. I'll put together a PR for moving from nothing to warn and we can add back log item for stricter enforcement (since that'll also require removing all the existing TODO/FIXME comments currently in the code bases)