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

Add some ruff checks #68

Merged
merged 4 commits into from
Jan 7, 2025
Merged

Add some ruff checks #68

merged 4 commits into from
Jan 7, 2025

Conversation

Djailla
Copy link

@Djailla Djailla commented Dec 19, 2024

No description provided.

@Djailla
Copy link
Author

Djailla commented Dec 19, 2024

There is an issue with cairo, not related to my PR :(

@Arno500
Copy link
Contributor

Arno500 commented Dec 26, 2024

You need to rebase to main so we can run the tests ;)
EDIT: Just did it through the UI, should be OK

@anisse
Copy link
Contributor

anisse commented Dec 30, 2024

Hello @Djailla in hwbench we test every commit, so appending a commit to fix tests won't work, you need to fix the faulty commits.

@Djailla Djailla force-pushed the main branch 2 times, most recently from f15490c to 8178e60 Compare January 2, 2025 10:20
@anisse
Copy link
Contributor

anisse commented Jan 6, 2025

For the last commit, did you want to enable pyupgrade lints "UP" (I think the commit message is not what you intended).

Also, since the ruff lints are now selected, we are losing the lints "E4", "E7" and "E9" from the ruff defaults: https://docs.astral.sh/ruff/configuration/ & https://github.com/astral-sh/ruff/blob/d45c1ee44f53d9c606085e79da1830b558fc001f/crates/ruff_workspace/src/options.rs#L739

Copy link
Contributor

@anisse anisse left a comment

Choose a reason for hiding this comment

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

LGTM

@anisse anisse merged commit 2c941d5 into criteo:main Jan 7, 2025
4 checks passed
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.

3 participants