Skip to content

Silent B028 from flake8-bugbear#55

Merged
msmdev merged 5 commits intomainfrom
silent-B028
Mar 7, 2023
Merged

Silent B028 from flake8-bugbear#55
msmdev merged 5 commits intomainfrom
silent-B028

Conversation

@msmdev
Copy link
Copy Markdown
Owner

@msmdev msmdev commented Feb 21, 2023

No description provided.

@msmdev
Copy link
Copy Markdown
Owner Author

msmdev commented Feb 21, 2023

@michalk8, flake8-bugbear started throwing B028 after the merge of #54 (but not before, meaning that all CI tests in the fix/update-precommits branch passed before merging!). So I played around a bit by adding # noqa B28 to all warnings in question, but those were removed as unnecessary when running pre-commit...
In effect, after running pre-commit run --all-files, all my changes were reversed and the flake8-bugbear again throws B028. I assume there is an inconsistency in the pre-commit config? Anyhow, I'm confused that this didn't show up in fix/update-precommits before the merge... what am I missing here?

@msmdev msmdev requested a review from michalk8 February 21, 2023 16:30
@msmdev msmdev self-assigned this Feb 21, 2023
@msmdev msmdev added the bug Something isn't working label Feb 21, 2023
@msmdev
Copy link
Copy Markdown
Owner Author

msmdev commented Feb 21, 2023

Also pre-commit run --all-files graciously passes both here and in main, but linting is failing in CI.

@michalk8
Copy link
Copy Markdown
Collaborator

michalk8 commented Feb 21, 2023

I'd suggest removing this line: https://github.com/msmdev/pyGPCCA/blob/main/.github/workflows/ci.yml#L32 (the cache should be invalidated when pre-commits change).

@msmdev
Copy link
Copy Markdown
Owner Author

msmdev commented Feb 21, 2023

I'd suggest removing this line: https://github.com/msmdev/pyGPCCA/blob/main/.github/workflows/ci.yml#L32

What is the effect of this line, exactly?

@michalk8
Copy link
Copy Markdown
Collaborator

What is the effect of this line, exactly?

It's an alternative key, when it cannot be found here.

@msmdev
Copy link
Copy Markdown
Owner Author

msmdev commented Feb 21, 2023

What is the effect of this line, exactly?

It's an alternative key, when it cannot be found here.

So it's basically using a old cached version of the pre-commit-config.yaml?

@michalk8
Copy link
Copy Markdown
Collaborator

michalk8 commented Feb 21, 2023

What is the effect of this line, exactly?

It's an alternative key, when it cannot be found here.

So it's basically using a old cached version of the pre-commit-config.yaml?

Yes, it shouldn't be there. But looking closely at the run, no cache-hit happened, so it's something else.

However, I think the B028 should be fixed in the code; tried locally (with different Python versions), can't also seem to reproduce the error on CI.

UPDATE: Can reproduce after I remove ~/.cache/pre-commit.

@msmdev
Copy link
Copy Markdown
Owner Author

msmdev commented Feb 27, 2023

What is the effect of this line, exactly?

It's an alternative key, when it cannot be found here.

So it's basically using a old cached version of the pre-commit-config.yaml?

Yes, it shouldn't be there. But looking closely at the run, no cache-hit happened, so it's something else.

However, I think the B028 should be fixed in the code; tried locally (with different Python versions), can't also seem to reproduce the error on CI.

UPDATE: Can reproduce after I remove ~/.cache/pre-commit.

What do you mean by "the B028 should be fixed in the code"?
If I add # noqa B028 in the respective lines, they are removed by yesqa during linting... so I can't prevent flake8 from complaining.

@msmdev
Copy link
Copy Markdown
Owner Author

msmdev commented Feb 27, 2023

Anyhow, flake8 is only complaining during the GitHub workflow - locally flake8 is graciously passing without # noqa B028...

@msmdev
Copy link
Copy Markdown
Owner Author

msmdev commented Feb 27, 2023

@michalk8, ooook, ignoring B028 in tox.ini solved the problem, but I'm still quite upset why flake8 is issuing B028 during GitHub workflows and not locally. -.-

@msmdev
Copy link
Copy Markdown
Owner Author

msmdev commented Mar 7, 2023

@michalk8, ooook, ignoring B028 in tox.ini solved the problem, but I'm still quite upset why flake8 is issuing B028 during GitHub workflows and not locally. -.-

@michalk8, would you please review? Thank you!

Copy link
Copy Markdown
Collaborator

@michalk8 michalk8 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @msmdev !

@msmdev msmdev merged commit 09e817e into main Mar 7, 2023
@msmdev msmdev deleted the silent-B028 branch March 7, 2023 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants