search

tommilligan / flake8-fixme

Check for FIXME, TODO and other temporary developer notes.
https://pypi.org/project/flake8-fixme/
Apache License 2.0
18 stars 2 forks source link

Make check for Fixme-comments case-insensitive #189

Open HansBambel opened 2 years ago

HansBambel commented 2 years ago

Hey, loving this extension!

I just had something peculiar happen: I noticed that our Pipeline is passing even though my colleague added Todo comments. These lines were highlighted in PyCharm as well which made me wonder whether flake8-fixme is broken. After testing a bit around I realized that the extension only checks for TODO comments written in all-uppercase.

Wouldn't it be helpful to check for Todos irrespective of the case?

tommilligan commented 2 years ago

Could you provide a couple of examples of the comments you'd like to catch? I don't think making all the current words flake8-fixme checks for is feasible, as I can see some of them being used in a general sentences. For instance, I don't think we should lint the word hack in the below example:

# This is a bit of a hack, but it'll do until python 3.9
HansBambel commented 2 years ago

Sure! I think the words will be at the start of the comment anyway with a more detailed description afterwards. Therefore something like this should be caught:

And so on.

tommilligan commented 2 years ago

So my problem is:

the start of the comment

is actually fairly hard to define. Also, it's not correct for all use cases. How about the start of each line/paragraph/bullet point in a docstring, for example?

"""Short text.

Fixme is a keyword often used to indicate work. Variants:

- fixme
- Fixme
- FIXME
"""

As I mentioned above, I don't think we can catch all the words case insensitively everywhere. And I think limiting the position where the capitalised versions are caught would not be a good thing either.

Unless you have a more detailed idea of how to solve this, I'll tend towards closing this as won't implement?

HansBambel commented 2 years ago

How is it done right now? All comments are scanned for the exact mentioning of "FIXME"? In your above example I would say that those would be fine being flagged as "Fixme"s.

In the end, I was just confused as to why some Todos were not flagged as such. Maybe just mentioning in the usage section that it needs to be written in all caps would be helpful.