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

Add pre-commit configuration and workflow #164

Merged
merged 3 commits into from
Jul 16, 2024
Merged

Conversation

larsks
Copy link
Member

@larsks larsks commented Jul 12, 2024

Add a pre-commit 1 configuration in .pre-commit-config.yaml and a
corresponding GitHub workflow to run the checks on pull requests.

The pre-commit checks use ruff 2 for both linting and formatting.
Developers should integrate these checks into their local workflow by
installing pre-commit and running pre-commit install to enable the local
pre-commit hook.

Note that we have removed tox -e pep8 from the existing test workflow, since
there's no reason to run the linting as part of the Python version matrix.

@larsks larsks requested review from DanNiESh and tzumainn July 12, 2024 21:23
@larsks larsks self-assigned this Jul 12, 2024
@larsks larsks force-pushed the fix/linting branch 3 times, most recently from 45f4280 to 3cb8fdd Compare July 12, 2024 21:45
@larsks larsks requested a review from QuanMPhm July 13, 2024 02:50
Copy link

@QuanMPhm QuanMPhm left a comment

Choose a reason for hiding this comment

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

@larsks Haha. Thank you for this! The latest ruff-pre-commit release just came out yesterday. Should we bother to update the version used to 0.5.2?

@larsks
Copy link
Member Author

larsks commented Jul 15, 2024

Sure, since this hasn't merged yet. I'll update the PR in a couple of minutes.

Copy link
Contributor

@tzumainn tzumainn left a comment

Choose a reason for hiding this comment

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

  • is " really that preferred over '??
  • it feels like the README could use a note regarding checks that are run, and how to run them - is that something that could be added?

@larsks
Copy link
Member Author

larsks commented Jul 16, 2024

is " really that preferred over '??

Meh. The point of a formatter is to pick one and then use it so that all the code is consistent. That is, it's not so much about preference as it is about consistentency. If you integrate the formatter in your editor you don't even have to think about it 😄.

it feels like the README could use a note regarding checks that are run, and how to run them

Good thought; I will update the README.

@tzumainn
Copy link
Contributor

is " really that preferred over '??

Meh. The point of a formatter is to pick one and then use it so that all the code is consistent. That is, it's not so much about preference as it is about consistentency. If you integrate the formatter in your editor you don't even have to think about it 😄.

it feels like the README could use a note regarding checks that are run, and how to run them

Good thought; I will update the README.

Fair enough; I'll point out that Ironic seems to have picked the single quote over the double quote (and that's my random preference as well), but I don't really care enough to make you undo all the changes and push them the other way.

@larsks
Copy link
Member Author

larsks commented Jul 16, 2024

@tzumainn I've added a section to the README.

Copy link
Contributor

@tzumainn tzumainn left a comment

Choose a reason for hiding this comment

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

Ah, I see how it works - neat! It seems pretty strict but whatever.

Should pre-commit be put in test-requirements.txt?

@tzumainn
Copy link
Contributor

Ah, I see how it works - neat! It seems pretty strict but whatever.

Should pre-commit be put in test-requirements.txt?

And the answer is no, as you just explained to me! LGTM

@tzumainn
Copy link
Contributor

oh, except for the conflicts because I just merged the other thing

Add a pre-commit [1] configuration in `.pre-commit-config.yaml` and a
corresponding GitHub workflow to run the checks on pull requests.

The pre-commit checks use ruff [2] for both linting and formatting.
Developers should integrate these checks into their local workflow by
installing pre-commit and running `pre-commit install` to enable the local
pre-commit hook.

Note that we have removed `tox -e pep8` from the existing test workflow, since
there's no reason to run the linting as part of the Python version matrix.

[1]: https://pre-commit.com/
[2]: https://docs.astral.sh/ruff/
This commit consists of the changes introduced by using ruff to format all
the source files in the repository and the changes necessary to correct
any linting errors reported by ruff.
@larsks larsks merged commit ee78fe4 into CCI-MOC:master Jul 16, 2024
6 checks passed
@larsks larsks deleted the fix/linting branch July 16, 2024 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants