fix(toml)!: Disallow [lints] in virtual workspaces#13155
Conversation
This was missed with the initial `[lints]` implementation. While this is a breaking change, this is aligned with ones we've done in the past. A lot of times, we warn first. My hope is that isn't needed this time because - It only exists virtual workspaces so they aren't published - It is a nop to have this which is likely to be caught - This is so new that the number of people using it, and likely running into this case, is quite low.
|
r? @weihanglo (rustbot has picked a reviewer for you, use r? to override) |
| pub workspace: Option<TomlWorkspace>, | ||
| pub badges: Option<InheritableBtreeMap>, | ||
| pub lints: Option<InheritableLints>, | ||
| // when adding new fields, be sure to check whether `to_virtual_manifest` should disallow them |
There was a problem hiding this comment.
Wonder we have a way to catch this automatically.
There was a problem hiding this comment.
The best way to catch this would be to put it in the design of the structs.
The problem is this doesn't align well with serde to say "these fields depend on this other field being present". We'd likely have to do stuff like what we do with workspace inheritance where we deserialize twice between two structures and pick the more appropriate one. Bleh. This isn't why I haven't done a bigger fix yet.
|
What does |
|
@bors r+ |
|
☀️ Test successful - checks-actions |
Breaking change |
Update cargo 20 commits in 9787229614b27854cf73d57ffae430d7c1e6caa4..1aa9df1a5be205cce621f0bc0ea6062a5e22a98c 2023-12-06 02:29:23 +0000 to 2023-12-12 14:52:31 +0000 - crates-io: Add support for other 2xx HTTP status codes (rust-lang/cargo#13158) - Remove the deleted feature test_2018_feature from the test (rust-lang/cargo#13156) - refactor(schema): Remove reliance on cargo types (rust-lang/cargo#13154) - fix(toml)!: Disallow `[lints]` in virtual workspaces (rust-lang/cargo#13155) - Limit exported-private-dependencies lints to libraries (rust-lang/cargo#13135) - chore: update to gix-index@0.27.1 (rust-lang/cargo#13148) - Update curl-sys to bring in curl 8.5.0 (rust-lang/cargo#13147) - chore: downgrade to openssl v1.1.1 (rust-lang/cargo#13144) - fix: explicitly remap current dir by using `.` (rust-lang/cargo#13114) - Don't rely on mtime to test changes (rust-lang/cargo#13143) - refactor: Pull PackageIdSpec into schema (rust-lang/cargo#13128) - fix: Print rustc messages colored on wincon (rust-lang/cargo#13140) - Add a windows manifest file (rust-lang/cargo#13131) - Avoid writing CACHEDIR.TAG if it already exists (rust-lang/cargo#13132) - re-enable flaky tests thanks to update to `gix-config`. (rust-lang/cargo#11821) (rust-lang/cargo#13130) - fix bash completion in directory with spaces (rust-lang/cargo#13126) - test: re-ignore git auth tests for gitoxide (rust-lang/cargo#13129) - fix(toml): Disallow inheriting of dependency public status (rust-lang/cargo#13125) - re-enable previously disabled tests with Windows-specific fix (rust-lang/cargo#13117) - refactor: Clarify PackageId constructor names (rust-lang/cargo#13123)
|
I'm probably missing something, but what's the rationale for this? Surely it may be useful to also harmonize lints in a virtual workspace? |
Then one should use |
|
Ah, that distinction also confused me. I think it might be worth emphasizing it in the release notes if they're not immutable :) |
|
I posted #13425 |
docs(changelog): Clarify lints in virtual workspace error Inspired by a conversation at #13155
This was missed with the initial
[lints]implementation.While this is a breaking change, this is aligned with ones we've done in the past. A lot of times, we warn first. My hope is that isn't needed this time because