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

chore: add ruff to QA checks #239

Merged
merged 1 commit into from
May 4, 2023
Merged

chore: add ruff to QA checks #239

merged 1 commit into from
May 4, 2023

Conversation

maxrake
Copy link
Contributor

@maxrake maxrake commented May 3, 2023

ruff is the hot new linter for Python. It is written in Rust and is extremely fast. It also has tremendous support and constant updates. It replaces some of the existing (pyupgrade) and planned (e.g., bandit, flake8, isort, and pylint) QA tools that are outlined in #14. More new rules are added all the time. The initial configuration enables ALL rules by default and then selectively ignores specific rules based on conflicts, preference, and existing issues in the backlog.

Using ALL has the risk that new rules may be enabled when ruff is updated but updates are automated, with a PR that includes enforced QA checks, to weekly dependency and pre-commit hook bumps. Therefore, the real risk is that a developer on this project may have a local version of ruff installed that is newer than the one used for the pre-commit hook. That should limit the exposure to no more than a week of "early" rules and may still result in improved code quality.

The list of actions taken in this PR include:

  • Add a custom ruff configuration in pyproject.toml
    • All rules are enabled by default
    • Specific rules and rule sets are disabled
  • Remove the pyupgrade pre-commit hook
    • Those checks are covered by ruff
  • Add the ruff pre-commit hook
    • This will enforce the rules in the QA check
  • Enable GitHub PR annotations for any findings
  • Add noqa comments, with reasons, where appropriate
  • Sort imports based on the isort configuration
  • Adhere to PEP-257 docstrings everywhere
  • Update the CONTRIBUTING guide to discuss QA expectations
  • Make updates to resolve all outstanding findings
  • Add a black badge to the README to show the formatting style used

Related to #14

`ruff` is the hot new linter for Python. It is written in Rust and is
extremely fast. It also has tremendous support and constant updates. It
replaces some of the existing (`pyupgrade`) and planned (e.g., `bandit`,
`flake8`, `isort`, and `pylint`) QA tools that are outlined in #14. More
new rules are added all the time. The initial configuration enables
`ALL` rules by default and then selectively ignores specific rules based
on conflicts, preference, and existing issues in the backlog.

Using `ALL` has the risk that new rules may be enabled when `ruff` is
updated but updates are automated, with a PR that includes enforced QA
checks, to weekly dependency and pre-commit hook bumps. Therefore, the
real risk is that a developer on this project may have a local version
`ruff` installed that is newer than the one used for the `pre-commit`
hook. That should limit the exposure to no more than a week of "early"
rules and may still result in improved code quality.

The list of actions taken in this PR include:

* Add a custom `ruff` configuration in `pyproject.toml`
  * All rules are enabled by default
  * Specific rules and rule sets are disabled
* Remove the `pyupgrade` pre-commit hook
  * Those checks are covered by `ruff`
* Add the `ruff` pre-commit hook
  * This will enforce the rules in the QA check
* Enable GitHub PR annotations for any findings
* Add `noqa` comments, with reasons, where appropriate
* Sort imports based on the `isort` configuration
* Adhere to PEP-257 docstrings everywhere
* Update the `CONTRIBUTING` guide to discuss QA expectations
* Make updates to resolve all outstanding findings
* Add a `black` badge to the `README` to show the formatting style used
@maxrake maxrake self-assigned this May 3, 2023
@maxrake maxrake requested a review from a team as a code owner May 3, 2023 23:02
@maxrake maxrake requested a review from andreaphylum May 3, 2023 23:02
Copy link

@andreaphylum andreaphylum left a comment

Choose a reason for hiding this comment

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

Looks good to me. All the lint-related changes are simple enough and don't change the semantics.

src/phylum/ci/ci_azure.py Show resolved Hide resolved
src/phylum/ci/ci_base.py Show resolved Hide resolved
@maxrake maxrake merged commit 188e244 into main May 4, 2023
@maxrake maxrake deleted the ruff_stuff branch May 4, 2023 14:27
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.

2 participants