On 2024.09.13 19:01, brian m. carlson wrote: > On 2024-09-07 at 15:15:12, Sean Allred wrote: > > Calvin Wan <calvinwan@xxxxxxxxxx> writes: > > > Add libgitrs, libgitrs-sys, libgitrs-test, and libgitrs-sys-test targets > > > to their respective Makefiles so they can be built and tested without > > > having to run cargo build/test. > > > > I feel like clippy should be run as part of these somehow, but I'm not > > sure where. > > Yes, that seems like a good idea in CI. > > > > +libgitrs-sys: > > > + $(QUIET)(\ > > > + cd contrib/libgit-rs/libgit-sys && \ > > > + cargo build \ > > > + ) > > > +.PHONY: libgitrs > > > +libgitrs: > > > + $(QUIET)(\ > > > + cd contrib/libgit-rs && \ > > > + cargo build \ > > > + ) > > > > We should definitely be setting `RUSTFLAGS=-Dwarnings` as an analog to > > `-Wall` in the C world, no? These crates should build without warnings. > > I believe -Dwarnings turns warnings into errors (at least it does in my > tests), which is equivalent to -Werror. We don't want that because it > breaks compiling older code with newer versions of the compiler, which > makes it harder to bisect changes or compiler on the system compiler (or > sometimes, other architectures or OSes). > > That would be fine for clippy, however, because that would only run in > CI, where we _would_ want to catch newer changes, but we want to > compile nonetheless. > -- > brian m. carlson (they/them or he/him) > Toronto, Ontario, CA Sorry, I missed your message before replying on this point last week. I'll remove this from our build.rs in V4.
