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

refactor: Remove stale references to flake8 #1625

Merged

Conversation

romanc
Copy link
Contributor

@romanc romanc commented Sep 2, 2024

Description

In PR #1466, flake8, black, and isort were replaced with the ruff linter and formatter. It seems there are a few leftovers of this transition. In this PR, I did a quick search for flake8 and deleted / replaced all instances found.

Removing flake8 means rebuilding the requirements (to propagate the changes to the frozen requirements-* files), which updates a couple of dependencies.

We also decided to remove the linter-* tasks of tox since they have been replaced by pre-commit.

Requirements

  • All fixes and/or new features come with corresponding tests: N/A
  • Important design decisions have been documented in the approriate ADR inside the docs/development/ADRs/ folder: N/A

If this PR contains code authored by new contributors please make sure:

  • The PR contains an updated version of the AUTHORS.md file adding the names of all the new contributors.

tox.ini Show resolved Hide resolved
@romanc romanc changed the title refacor: Remove stale references to flake8 refactor: Remove stale references to flake8 Sep 2, 2024
@egparedes
Copy link
Contributor

Thanks for contributing with this PR to clean up GT4Py!

@egparedes / @havogt: How can I propagate changes from requirements-dev.in to auto-generated files like requirements-dev.txt?

Just run: tox r -e requirements-base. More details here: https://github.com/GridTools/gt4py/blob/main/docs/development/tools/requirements.md

@romanc romanc marked this pull request as ready for review September 2, 2024 16:17
@romanc
Copy link
Contributor Author

romanc commented Sep 2, 2024

@egparedes thanks for the quick reply and the pointers. I could run the tox command to update the requirements and I checked the mypy version for consistency. This should be ready for review now.

/cc @FlorianDeconinck

@romanc romanc requested a review from egparedes September 2, 2024 16:18
@romanc romanc force-pushed the romanc/remove-stale-references-to-flake8 branch from fec69b2 to 6cbec77 Compare September 2, 2024 16:23
@egparedes
Copy link
Contributor

egparedes commented Sep 3, 2024

I hadn't realized that the CI checks were waiting for explicit approval of a member of the developers' group and didn't run automatically. I've triggered them now and as soon as they finish I'll approve the PR. In the meantime, please merge back the changes from main in this PR to make sure everything works fine.

Roman Cattaneo added 6 commits September 3, 2024 10:07
Flake8 was removed in the past in favor of the ruff [1] linter and
formatter.

[1] https://astral.sh/ruff
linters run as part of pre-commit and don't need to re-run as part of
tox tests.
- ran `tox r -e requirements-base`
- checked (and fixed) the mypy mirror version in .pre-commit-config.yaml
This commit runs `pre-commit run -a` to run the new version of `ruff` and
`ruff-format` on all files. This changes some examples and notebooks.
@romanc romanc force-pushed the romanc/remove-stale-references-to-flake8 branch from 6b61804 to 152981b Compare September 3, 2024 08:07
@romanc
Copy link
Contributor Author

romanc commented Sep 3, 2024

@egparedes Thanks for starting the tests. I rebased my branch on top of main. Looks like this stopped the ongoing CI and now tests are waiting again for approval to run. I guess you'll need to manually start them again :/

@egparedes
Copy link
Contributor

cscs-ci run default

Copy link
Contributor

@egparedes egparedes 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.

@egparedes egparedes merged commit e4fb802 into GridTools:main Sep 3, 2024
55 checks passed
@egparedes
Copy link
Contributor

@romanc Thanks again for your contribution. I've approved and already merged the PR myself because I want to add these changes to another PR I'm working on.

@romanc romanc deleted the romanc/remove-stale-references-to-flake8 branch September 3, 2024 12:50
@romanc
Copy link
Contributor Author

romanc commented Sep 3, 2024

Thanks for merging @egparedes. Happy to contribute!

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