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

Apply ruff #34

Merged
merged 5 commits into from
Feb 22, 2024
Merged

Apply ruff #34

merged 5 commits into from
Feb 22, 2024

Conversation

ndaelman-hu
Copy link
Contributor

No description provided.

@ndaelman-hu ndaelman-hu self-assigned this Feb 22, 2024
@ndaelman-hu ndaelman-hu added the format Update the formatting style label Feb 22, 2024
@ndaelman-hu
Copy link
Contributor Author

So, 1 question is about the pycodestyle and pylint dependencies: can these dependencies be removed with ruff present?

@blueraft
Copy link
Collaborator

blueraft commented Feb 22, 2024

So, 1 question is about the pycodestyle and pylint dependencies: can these dependencies be removed with ruff present?

Yes, they should be removed. You can remove the pycodestyle and pylint step in the workflow file too.

Currently the workflow file only has a ruff lint check but no format check so you might want to add one for that too. This can be added the following way:
https://github.com/nomad-coe/nomad-parser-example/blob/2bd4d49996b3d300c87d2225c5433c41e8eb7ce7/.github/workflows/actions.yaml#L49C1-L55C29

Folks at astral are working on having a unified command for both lint and format but it's not done yet - astral-sh/ruff#8232.

@ndaelman-hu
Copy link
Contributor Author

So, 1 question is about the pycodestyle and pylint dependencies: can these dependencies be removed with ruff present?

Yes, they should be removed. You can remove the pycodestyle and pylint step in the workflow file too.

Currently the workflow file only has a ruff lint check but no format check so you might want to add one for that too. This can be added the following way: https://github.com/nomad-coe/nomad-parser-example/blob/2bd4d49996b3d300c87d2225c5433c41e8eb7ce7/.github/workflows/actions.yaml#L49C1-L55C29

Folks at astral are working on having a unified command for both lint and format but it's not done yet - astral-sh/ruff#8232.

In the link you provided, ruff formatting is still commented out, though...

@blueraft
Copy link
Collaborator

So, 1 question is about the pycodestyle and pylint dependencies: can these dependencies be removed with ruff present?

Yes, they should be removed. You can remove the pycodestyle and pylint step in the workflow file too.
Currently the workflow file only has a ruff lint check but no format check so you might want to add one for that too. This can be added the following way: https://github.com/nomad-coe/nomad-parser-example/blob/2bd4d49996b3d300c87d2225c5433c41e8eb7ce7/.github/workflows/actions.yaml#L49C1-L55C29
Folks at astral are working on having a unified command for both lint and format but it's not done yet - astral-sh/ruff#8232.

In the link you provided, ruff formatting is still commented out, though...

Yeah 😅. Markus wanted auto-formatting to be optional in the example templates, but we are opting-in to auto-format code in this repo.

@ndaelman-hu
Copy link
Contributor Author

@ladinesa If this fixes the pipeline, then we should consider adding merge checks again, i.e. you can only merge when passing the pipeline.
I would also split the build test from the python tests...

@blueraft
Copy link
Collaborator

LGTM from my side, you can also remove the pylintrc file.

@ndaelman-hu ndaelman-hu merged commit 3c1439d into master Feb 22, 2024
3 checks passed
@ndaelman-hu
Copy link
Contributor Author

LGTM from my side, you can also remove the pylintrc file.

Applied it in a separate merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
format Update the formatting style
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants