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

Adopt inital noop ruff linter #5623

Merged
merged 3 commits into from
Dec 11, 2023
Merged

Adopt inital noop ruff linter #5623

merged 3 commits into from
Dec 11, 2023

Conversation

bjlittle
Copy link
Member

@bjlittle bjlittle commented Dec 11, 2023

🚀 Pull Request

Description

This pull-request adds the configuration for a no-op ruff linter i.e., the ruff linter runs but all rules are ignored, hence there are no compliance exceptions raised.

Note that, it adds the pre-commit hook to ensure that ruff (and it's configured rules) are applied for developers using pre-commit and on pull-requests via the pre-commit.ci service.

Also note that ALL linter rules have been selected in the pyproject.toml, and each of the currently existing 58 ruff linter rules (see here) have been ignored in the .ruff.toml configuration file, which extends the ruff configuration defined in the pyproject.toml.

As a strategy going forwards, I'd recommend that we:

  • add permanent rule and file exceptions to the pyproject.toml
  • use the .ruff.toml to collate all temporary rule and file exceptions, but aim to address these all over time.

Ultimately, the goal would be to have no temporary exceptions in the .ruff.toml configuration file. Also, we should always aim to minimize our permanent (documented) exceptions.

We can also slowly drop use of all the third-party packages that ruff replaces, namely:

  • black
  • flake8
  • isort

Taking the above approach will allow us to adopt and become more compliant with ruff over time, at our own pace.

This strategy is open-ended, in that we have enabled ALL linter rules for ruff (with explicit ignores) so that we automatically adopt new non-preview linter rules by default. If new rules are implemented within ruff that break us, then either we address them straight away or add temporary/permanent exceptions as we deem fit.


Consult Iris pull request check list

Copy link

codecov bot commented Dec 11, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1e1a215) 89.69% compared to head (b0e9b0f) 89.69%.

❗ Current head b0e9b0f differs from pull request most recent head b32bedb. Consider uploading reports for the commit b32bedb to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5623   +/-   ##
=======================================
  Coverage   89.69%   89.69%           
=======================================
  Files          90       90           
  Lines       22807    22807           
  Branches     5441     5441           
=======================================
  Hits        20456    20456           
  Misses       1618     1618           
  Partials      733      733           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tkknight tkknight self-requested a review December 11, 2023 17:05
Copy link
Contributor

@tkknight tkknight left a comment

Choose a reason for hiding this comment

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

This looks great @bjlittle. It enables ruff but we can adopt the rules over time.

@tkknight tkknight merged commit c11d8bb into SciTools:main Dec 11, 2023
14 checks passed
@bjlittle bjlittle mentioned this pull request Dec 11, 2023
13 tasks
tkknight added a commit to tkknight/iris that referenced this pull request Dec 13, 2023
* main:
  Adopt inital noop ruff linter (SciTools#5623)
  Update links to https (SciTools#5621)
  Bump actions/stale from 8 to 9 (SciTools#5616)
  Faster and simpler iris.util.array_equal (SciTools#5610)
  Updated environment lockfiles (SciTools#5608)
  Bump actions/setup-python from 4 to 5 (SciTools#5614)
  Possible citation updates -- not clear whether appropriate. (SciTools#5453)
  Whats new updates for v3.7.0 . (SciTools#5451)
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