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 code linting and formatting checking GitHub Action #650

Draft
wants to merge 10 commits into
base: dev
Choose a base branch
from

Conversation

garlic-os
Copy link
Contributor

@garlic-os garlic-os commented Jun 3, 2024

Description

This pull request adds a GitHub Action workflow to automatically run linting and format checking on pull requests. The workflow is configured to do this with ruff and ensure that all new code adheres to our coding standards before merging.

The workflow file .github/workflows/reusable-ruff.yml has been added from ASFHyP3/actions/#reusable-ruffyml.

Motivation and Context

I saw that the project already encourages the use of a formatter before submitting pull requests. This change enforces code formatting consistency adding a notice to your pull request if it does not have proper formatting.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • I have added an explanation of what your changes do and why you'd like us to include them.
  • I have written new tests for your core changes, as applicable.
  • I have successfully ran tests with your changes locally.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@garlic-os
Copy link
Contributor Author

Looks like I misunderstood the use of this tool. It actually does not push any code itself, but just checks to make sure that the code is all formatted already. In this case the repo will need to be formatted manually the first time.

@jhkennedy
Copy link
Collaborator

@garlic-os it might be worth checking out ruff, which includes a linter and formatter. It also allows you to only check new code going forward, so you don't have to reformat the whole codebase:
https://docs.astral.sh/ruff/tutorial/#adding-rules

We also provide a re-usable ruff action here:
https://github.com/ASFHyP3/actions/#reusable-ruffyml

@garlic-os garlic-os changed the title Add autopep8 GitHub Action Add code linting and formatting checking GitHub Action Jul 10, 2024
@garlic-os
Copy link
Contributor Author

garlic-os commented Jul 10, 2024

Thanks for the suggestion. I didn't know about ruff; it really looks like a better alternative to autopep8! I followed the instructions at ASFHyP3/actions/#reusable-ruffyml and put it in place of the autopep8 runner. @jhkennedy I trust you have some familiarity with it, would you know how to set it up to only check new code like you mentioned?

@jhkennedy
Copy link
Collaborator

@garlic-os it looks I didn't quite remember right; you can have ruff add noqa statements for things that currently flag but that will also end up with a PR that has a lot of changes. So it's probably best to fix what you can easily in a PR and then noqa the rest.

Alternatively, you can use a tool like darker:
astral-sh/ruff#4049 (reply in thread)

which will allow just the changes to be checked, but you won't be able to use the ASFHyP3/actions reusable action other than as a starting point for a new action.

@jlmaurer
Copy link
Collaborator

@garlic-os is this one ready for review?

@garlic-os garlic-os marked this pull request as draft July 17, 2024 18:00
@garlic-os
Copy link
Contributor Author

No, sorry. I meant to convert it back to a draft. I feel #672 should happen before this one now.

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