chore(lint): use dylint as lint driver for tfhe-lint #1933
Conversation
nsarlin-zama
force-pushed
the
ns/chore/dylint
branch
3 times, most recently
from
0d61a52 to
4da677f
Compare
nsarlin-zama
force-pushed
the
ns/chore/dylint
branch
from
4da677f to
72782fa
Compare
Makefile
Outdated
| --features=boolean,shortint,integer,zk-pok -- -D warnings | ||
| tfhe_lints: install_cargo_dylint | ||
| RUSTFLAGS="$(RUSTFLAGS)" cargo dylint --all -p tfhe --no-deps -- \ | ||
| --features=boolean,shortint,integer,zk-pok |
There was a problem hiding this comment.
Not really related to this PR, but strings feature should also be enabled here
There was a problem hiding this comment.
can still be added here that's a good remark
In general, everywhere we have the integer feature we may want to also have the strings feature
There was a problem hiding this comment.
We could have a ALL_TARGETS variable in the Makefile so that we only have to update it when adding new features
There was a problem hiding this comment.
targets has a different meaning for cargo, there is already --all-targets so it's a bit confusing
nsarlin-zama
force-pushed
the
ns/chore/dylint
branch
3 times, most recently
from
9b669c1 to
47e3257
Compare
nsarlin-zama
force-pushed
the
ns/chore/dylint
branch
from
47e3257 to
48d9d23
Compare
| @@ -416,10 +415,15 @@ clippy_versionable: install_rs_check_toolchain | |||
| RUSTFLAGS="$(RUSTFLAGS)" cargo "$(CARGO_RS_CHECK_TOOLCHAIN)" clippy --all-targets \ | |||
| -p tfhe-versionable -- --no-deps -D warnings | |||
|
|
|||
| .PHONY: clippy_tfhe_lints # Run clippy lints on tfhe-lints | |||
| clippy_tfhe_lints: install_cargo_dylint # the toolchain is selected with toolchain.toml | |||
There was a problem hiding this comment.
toolchain auto installs itself ?
There was a problem hiding this comment.
yes, you can see it here: https://github.com/zama-ai/tfhe-rs/actions/runs/12713445967/job/35441362205?pr=1933#step:5:3134
closes: https://github.com/zama-ai/tfhe-rs-internal/issues/854