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

Crate imports_granularity requires nightly #395

Closed
maspe36 opened this issue Apr 29, 2024 · 4 comments
Closed

Crate imports_granularity requires nightly #395

maspe36 opened this issue Apr 29, 2024 · 4 comments

Comments

@maspe36
Copy link
Collaborator

maspe36 commented Apr 29, 2024

This was identified by @luca-della-vedova here.

Currently, our CI doesn't actually check that our imports are formatted with crate level import granularity. Additionally, this functionality is an unstable feature (rust-lang/rustfmt#4991) so it's only supported in nightly.

Running locally without a nightly toolchain

ros2_rust/rclrs$ cargo fmt
Warning: can't set `imports_granularity = Crate`, unstable features are only available in nightly channel.

The way I see it, we have two options.

  1. We manually check imports and try to catch this in PRs
  2. We start using a nightly toolchain in our CI. We can mitigate some of the churn for nightly here by selecting a version ourselves.
@Guelakais
Copy link
Contributor

Guelakais commented Apr 30, 2024

Isn't this something that should be mentioned directly in the readmy or .rustfmt.toml as a comment? The Cargo manifest format allows comments starting with #.

@esteve
Copy link
Collaborator

esteve commented Apr 30, 2024

@maspe36 I prefer to only target stable for builds, however we could target nightly for formatting, but I wonder how much of a moving target it is.

@mxgrey
Copy link
Collaborator

mxgrey commented Apr 30, 2024

#396 should get us what we want. It will continue to build and test the repo using the stable channel but use nightly when checking the format.

@maspe36
Copy link
Collaborator Author

maspe36 commented Apr 30, 2024

Ah yeah, I didn't word the second option well but #396 is exactly what I was referring to. We can close once that's merged.

@maspe36 maspe36 closed this as completed Jul 13, 2024
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

4 participants