-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Apply code style to project #3231
Conversation
- remove upper version caps - updated the minimum version of most of Pelican's runtime deps - replaced black with ruff as a formatter for pelican - added a cache step to the docs CI task so that the docs can be downloaded and inspected.
Assuming the SHA doesn't change for the commit titled Apply code style to project via: ruff format ., before this PR is merged, the following commit hash should be added to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ship it!
Oof, the double quotes. I don't object, but this really rules out any automated resolution of any existing PRs. But, I approve of forward motion! |
pyproject.toml
Outdated
"flake8>=6.1.0", | ||
"flake8-import-order>=0.18.2", | ||
"invoke>=2.2.0", | ||
"isort>=5.12.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't isort
be removed then?
"isort>=5.12.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe so, and for that matter I think Flake8 and Black should also be removed, right @offbyone?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not @offbyone, but I don't see black or isort in .pre-commit-config.yaml, so I agree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct; I left them in on the chance that someone ran them by hand, but yeah, they should be yeeted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Many thanks to @boxydog, @offbyone, @lioman, and @pauloxnet for helping with this endeavor. Much appreciated! 🤗 |
downloaded and inspected.
Resolves #3217
Resolves #3227
Closes #3123