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

Linting rules aren't enforced the same as format rules #1742

Closed
micahellison opened this issue May 20, 2023 · 1 comment · Fixed by #1763
Closed

Linting rules aren't enforced the same as format rules #1742

micahellison opened this issue May 20, 2023 · 1 comment · Fixed by #1763
Labels
build Issues related to the build pipeline 📌 This can't go stale

Comments

@micahellison
Copy link
Member

Diagnostic output

n/a - this is a dev issue

Current Behavior

Our linter isn't consistently checking for the same rules that black is running.

Expected Behavior

We expect to catch formatting issues in a poe test run or through C/I, rather than always having to remember to run poe format.

Repro Steps

One rule that isn't consistent is the line length rule. Here's how to repro:

Add a line to a .py file in the jrnl folder like so:

print("Testing a very long line") # here is a long enough line with a comment at the end, so black will indent it

Run poe lint

It will pass, but it shouldn't.

Run poe format and it will change into a state that should pass:

print(
    "Testing a very long line"
)  # here is a long enough line with a comment at the end, so black will indent it

Debug output

n/a

Other Information

Might be worth switching to a different linter. Maybe ruff, which runs much faster than flakeheaven.

@micahellison micahellison added bug Something isn't working 🆕 New! build Issues related to the build pipeline 📌 This can't go stale and removed 🆕 New! labels May 20, 2023
@micahellison
Copy link
Member Author

I'm playing around with ruff and I like it, however its docs mention that it omits "those stylistic rules made obsolete by the use of an autoformatter, like Black".

The real solution to this issue is to add black --check to our linting step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues related to the build pipeline 📌 This can't go stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant