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

ci: Replace flake8 with Ruff #1587

Merged
merged 1 commit into from
Jan 29, 2023
Merged

ci: Replace flake8 with Ruff #1587

merged 1 commit into from
Jan 29, 2023

Conversation

rht
Copy link
Contributor

@rht rht commented Jan 21, 2023

See https://notes.crmarsh.com/ruff-the-first-200-releases. Highlight:

Probably the most complex and rewarding deployment (at least for me) thus far came from Dagster, where they replaced 70 parallel CI jobs (to run Pylint) with a single pre-commit hook. It runs in 400 milliseconds, a ~1000x speed-up over their previous setup.
It’s a perfect encapsulation of the impact that “faster tools” can have on a project, on a team, on a company. Ruff isn’t just faster; it’s so much faster that it completely changes the workflow.

@wang-boyu we should do this for Mesa-Geo. @EwoutH check this out.

@codecov
Copy link

codecov bot commented Jan 21, 2023

Codecov Report

Base: 82.06% // Head: 82.06% // No change to project coverage 👍

Coverage data is based on head (dbb7e1e) compared to base (e3cc4d4).
Patch coverage: 100.00% of modified lines in pull request are covered.

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

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1587   +/-   ##
=======================================
  Coverage   82.06%   82.06%           
=======================================
  Files          18       18           
  Lines        1388     1388           
  Branches      271      271           
=======================================
  Hits         1139     1139           
  Misses        205      205           
  Partials       44       44           
Impacted Files Coverage Δ
mesa/visualization/UserParam.py 95.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rht
Copy link
Contributor Author

rht commented Jan 21, 2023

It took 2 s to run flake8 on the codebase, and 1 s for Black. Now GH Actions says 0 s. Also, it is a single linter instead of separate. We should configure further to include isort etc.

@rht
Copy link
Contributor Author

rht commented Jan 21, 2023

I think it doesn't replace Black yet. It is meant to be used alongside Black.

@rht rht changed the title ci: Replace flake8 & Black with Ruff ci: Replace flake8 with Ruff Jan 21, 2023
@tpike3
Copy link
Member

tpike3 commented Jan 21, 2023

This LGTM, Ruff looks established (used by heavily supported libraries e.g. pandas) and reliable (large number of contributors so low bus factor). however as it is a significant dependency change, could I get a second @wang-boyu, @jackiekazil @EwoutH and then we can merge.

@wang-boyu
Copy link
Member

Looks great! Probably there'll be more tools written in Rust in the future. I'll update Mesa-Geo with the same.

@rht
Copy link
Contributor Author

rht commented Jan 26, 2023

Once this is merged, I will add more as configured in Zulip.

@wang-boyu
Copy link
Member

LGTM.

@tpike3 tpike3 merged commit 912dfbf into projectmesa:main Jan 29, 2023
@jackiekazil
Copy link
Member

Ty @rht

@jackiekazil jackiekazil added this to the v1.2.0 Taylor milestone Feb 27, 2023
@jackiekazil jackiekazil mentioned this pull request Mar 7, 2023
4 tasks
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.

4 participants