Skip to content

Lint non-workspace crates in CI #268

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Jun 24, 2025

Do a bunch of preparation and then add commands to the already present lint job in CI to lint verify and integration_tests.

Close: #236

We changed the versions when adding support for v29 but did not fix this
doc.
`verify` is intentionally not part of the workspace. Add a script to
lint it.
@tcharding
Copy link
Member Author

tcharding commented Jun 24, 2025

If/when this merges we should run the formatter as well. And also investigate #246

justfile Outdated
@@ -18,6 +21,14 @@ check:
# Lint everything.
lint:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
lint:
lint: lint-verify lint-integration-tests

The other tests need to be listed as dependencies. It doesn't run as it currently is and I had to look up why: If you list them below it's run in the shell so would need to be just lint-verify.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah nice, thanks man.

@0xZaddyy
Copy link
Contributor

Looking at this now, I clearly see the mistakes I made earlier and I really appreciate how clean and well structured your commits are it's a great reference for improving mine.

Add commands and alias' to lint `verify` and `intergration-tests`.
Use `matches!` instead of matching manually. Found by clippy.
Simplify and put the positive bool first and the negative second.

E.g `if this && !that`

Because it reads a bit easier.

Internal change only, no logic change.

Clears a clippy warning.
Lint the `verify` and `integration_tests` crates in CI using the scripts
we already have.
@tcharding tcharding force-pushed the 06-24-ci-lint-non-workspace-crates branch from dd5ea03 to 2858e46 Compare June 25, 2025 01:10
@tcharding
Copy link
Member Author

Thanks @0xZaddyy, glad you found it useful. Stick at it man, its a long path.

@tcharding tcharding requested a review from jamillambert June 25, 2025 01:16
Copy link
Collaborator

@jamillambert jamillambert left a comment

Choose a reason for hiding this comment

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

ACK 2858e46

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

verify and integration_test are not linted in CI
3 participants