search

conda-forge / conda-smithy

The tool for managing conda-forge feedstocks.
https://conda-forge.org/
BSD 3-Clause "New" or "Revised" License
146 stars 170 forks source link

Introduce Ruff Linter #1919

Closed ytausch closed 2 days ago

ytausch commented 2 months ago

Checklist

This PR introduces ruff as a source code linter, which will ensure a consistent and modern code style.

Please see the commits to check which fixes where applied manually and which automatically.

ytausch commented 2 months ago

@beckermr What do you mean by "expanding the ruleset"?

My PR already contains more rulesets than the default (and more than you propose):

https://github.com/conda-forge/conda-smithy/blob/12f4a1fa107b75069d5380e166b2d5d41cbb6ed9/pyproject.toml#L36-L52

Also, what is the rationale behind your proposal to enable preview mode? Preview mode contains unstable features, which is not something we want here IMO.

beckermr commented 2 months ago

Ah I missed that! LGTM!

jaimergp commented 2 months ago

Awesome! I suggest that when we merge this one, we do it via Squash, so then we can easily omit this PR in the blame view via that github file whose name I can't remember :)

jaimergp commented 2 months ago

Any thoughts about the rationale for the preview mode exposed in https://github.com/conda-forge/conda-forge-webservices/pull/616?

beckermr commented 2 months ago

I like all of the flake8 rules so I like preview mode, but that is me.

ytausch commented 2 months ago

I would be fine with enabling preview mode if you think that's necessary - is there anything else you want to debate about this PR? All open discussions are solved from my side I want this to get merged.

ytausch commented 2 months ago

@conda-forge/core

ytausch commented 1 month ago

@h-vetinari @beckermr @jaimergp @jakirkham Can we please get this through?

beckermr commented 1 month ago

Let's wait for a few more reviews and then merge.

ytausch commented 3 days ago

@jaimergp

ytausch commented 3 days ago

Rebasing this over and over again is quite tedious so I'll wait until we have found a consensus about the rules we want to enforce and then I can do it one last time. Happy if we finally merge this after almost 3 months.

beckermr commented 3 days ago

We have two approvals from core , so I'd be happy to merge today.

ytausch commented 2 days ago

To make rebasing easier and the commit history cleaner, I reran ruff with the final configuration, and reapplied everything we have discussed here. From my side, this should be fine now.

beckermr commented 2 days ago

We have two test failures. After those are fixed, we can merge.

ytausch commented 2 days ago

Done.

wolfv commented 2 days ago

Awesome job, @ytausch