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

Fix clippy lints, or are they being ignored on purpose? #338

Open
bzm3r opened this issue Jan 22, 2024 · 2 comments
Open

Fix clippy lints, or are they being ignored on purpose? #338

bzm3r opened this issue Jan 22, 2024 · 2 comments

Comments

@bzm3r
Copy link
Contributor

bzm3r commented Jan 22, 2024

There are some clippy warnings throughout the project.

Should these be fixed, or are they meant to be ignored on purpose?

(I can submit a PR with fixes, if they are meant to be fixed.)

@pacak
Copy link
Owner

pacak commented Jan 22, 2024

Hmm... I'm trying to keep it clean from clippy complains and formatting. Any ideas why CI isn't catching them?

As far as fixing them - _documentation.rs is generated and I'm removing it in the big rewrite branch (pushed as concrete2), some renames might make sense - I'll take a look at that. ShortLong::ShortLong can be ShortLong::Both I guess...

@vallentin
Copy link
Contributor

The CI actually is outputting all the warnings. The issue is that only the cargo doc check includes -D warnings.

- name: clippy minimal features
run: cargo clippy --workspace --all-targets --no-default-features
- name: clippy default features
run: cargo clippy --workspace --all-targets
- name: clippy all features
run: cargo clippy --workspace --all-targets --all-features
- name: test minimal features
run: cargo test -p bpaf -p bpaf_derive --all-targets --no-default-features --no-fail-fast
- name: test default features
run: cargo test -p bpaf -p bpaf_derive --all-targets --no-fail-fast
- name: test all features
run: cargo test -p bpaf -p bpaf_derive --all-targets --all-features --no-fail-fast
- name: Example documentation
run: cargo build --all-features -p docs2 && git diff && git diff-index --quiet HEAD --
- name: Rustdoc
env:
RUSTDOCFLAGS: -D warnings
run: cargo doc --all --no-deps --all-features

In short, the following change is needed for the clippy check(s), to catch warnings:

- cargo clippy --workspace --all-targets
+ cargo clippy --workspace --all-targets -- -D warnings

Adding -D warnings to the clippy check(s), should be enough to catch all the warnings.

Check out the workflow we're using for Askama.

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

No branches or pull requests

3 participants