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 the linebender lint set #369

Open
PoignardAzur opened this issue Aug 25, 2024 · 5 comments
Open

Add the linebender lint set #369

PoignardAzur opened this issue Aug 25, 2024 · 5 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@PoignardAzur
Copy link

PoignardAzur commented Aug 25, 2024

We should add a standardized set of rustc lints and clippy lints to all our projects. If we raise MSRV to 1.74, we can add them directly to Cargo.toml.

See zulip discussion for what our lint set should be. See linebender/peniko#47 for our current lint set.

Aside from the Cargo.toml lints, we should also add the following annotation to our crate root:

#![warn(clippy::print_stdout, clippy::print_stderr, clippy::dbg_macro)]

(We don't want a workspace-level lints for those because we want to tolerate prints in tests and examples.)

See this zulip thread for details.

We should add the following annotation at the crate roots:

We should also remove leftover prints, if any, or give them an opt-out.

@PoignardAzur PoignardAzur added the good first issue Good for newcomers label Aug 25, 2024
@PoignardAzur PoignardAzur changed the title Add clippy lints against prints Add the linebender lint set Aug 26, 2024
@PoignardAzur
Copy link
Author

@0xdevcollins
Copy link

Hi @PoignardAzur, Could I be assigned to this task?

@waywardmonkeys
Copy link
Contributor

@devcollinss No one is currently working on this, so feel free to submit self-contained patches for the various lints.

I have some reservations about some of the lints, so if something is a big change, feel free to talk with us about it!

Maybe a good structure is a draft PR that has a lot of commits in it so that we can review them separately but still squash it down when it is ready to hit main?

@0xdevcollins
Copy link

@PoignardAzur I just made a draft PR #391

@xStrom
Copy link
Member

xStrom commented Oct 17, 2024

If we raise MSRV to 1.74, we can add them directly to Cargo.toml.

We don't need to raise the MSRV to add them to Cargo.toml. Older toolchains will just issue a warning and that is fine. The warning does not trigger if Kurbo is used as a dependency.

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

No branches or pull requests

4 participants