search

JBKahn / flake8-print

flake8
MIT License
121 stars 21 forks source link

#50: remove noqa detection since its included in flake8 #51

Closed Melevir closed 2 years ago

Melevir commented 3 years ago

This one will make flake8-print works fine with modules like flake8-noqa. It will also make it work fine for several edge-cases (like this one: https://github.com/PyCQA/flake8/issues/343#issuecomment-961128319)

JBKahn commented 3 years ago

Is it possible to remove it and still have the tests pass?

Melevir commented 3 years ago

Nope. This functionality is implemented in flake8 repo and they have tests for that, so it's not required to test it in each plugin :)

JBKahn commented 3 years ago

Agreed, but are the tests passing before you remove them?

We don't need to keep them but they should be passing if it's built in before we remove them? Also, I don't think it used to work. Is there a version or change where this occurred? Should we keep it for some legacy version and do a flake8 version check?

Melevir commented 3 years ago

For me even existing tests are failing now. I think it is worth fixing and introducing Github Actions for CI, but not in this pr.

#noqa functionality arrived in flake8 in 3.0.0, so it was 6 years ago. I personally don't think, that it can cause backward compatibility issues.

JBKahn commented 3 years ago

That seems to contradict the issue here: https://github.com/JBKahn/flake8-print/issues/40

So I'm going to have to look into this. Perhaps the original issue opened was wrong or there was some other cause but I want to understand a bit more. I don't want to make the change until I have all the context.

JBKahn commented 3 years ago

Ahhh I see the bug he identified wasn't opposed to the fix. But instead my code. I'm on vacation (have been for a week) and I'll test this out when I get back next week and am not on my mobile device.

Zac-HD commented 2 years ago

@JBKahn - anything I can do to help with this?

JBKahn commented 2 years ago

Nah, I'll get to it. The whole thing slipped my mind. The following day after I posted that I got a burn in the kitchen and just forgot. Thanks for the bump.

Zac-HD commented 2 years ago

Bump again? 😜

JBKahn commented 2 years ago

I'm good now but that burn did take over a month to heal which had me out of commission for a while. I'll try to get this through tomorrow.

Zac-HD commented 2 years ago

Another bump 😋 - hope you're fully recovered 🤗

(last one, unless you tell me to keep going)

JBKahn commented 2 years ago

I pushed it to pypi.

Lots of stuff going on, I was sick again between but thanks for the pushes.