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

Fail for non-formatted code #1274

Merged
merged 4 commits into from
Apr 16, 2024

Conversation

zvi-quantivly
Copy link
Contributor

Updates the GitHub Actions workflow to fail if the code isn't formatted. This PR depends on #1273.

@oesteban oesteban marked this pull request as ready for review April 16, 2024 21:15
@oesteban oesteban merged commit 9d5e578 into nipreps:master Apr 16, 2024
8 of 11 checks passed
@@ -81,7 +81,7 @@ jobs:
- uses: actions/checkout@v4
- name: Set up Python
uses: actions/setup-python@v5
- run: pipx run ruff check mriqc/
- run: pipx run ruff format --diff
Copy link
Contributor

Choose a reason for hiding this comment

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

This changed ruff check into ruff format. On purpose?

Copy link
Member

Choose a reason for hiding this comment

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

I had missed it. Does format include check?

Copy link
Contributor

Choose a reason for hiding this comment

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

No , ruff check and ruff format are different. I can add ruff check back in #1348.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, do we want to apply ruff to mriqc/ only, or to the whole repository? I think applying ruff to the whole repository was a good move.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for this, yes, please include these suggestions within #1348.

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.

3 participants