Implement opt-in workspace overrides #812
Conversation
from manifest `(package.metadata.cargo-semver-checks.)?lints.workspace = true` keys
| let selected_manifest = manifest::Manifest::parse(selected.manifest_path.clone().into_std_path_buf())?; | ||
| // the key `lints.workspace` for the Cargo lints table | ||
| let lint_workspace_key = selected_manifest.parsed.lints.is_some_and(|x| x.workspace); | ||
| let metadata_workspace_key = package_overrides.as_ref().is_some_and(|x| x.workspace); | ||
|
|
||
| if lint_workspace_key || metadata_workspace_key { | ||
| if let Some(workspace) = &workspace_overrides { | ||
| overrides.push(Arc::clone(workspace)); | ||
| } |
There was a problem hiding this comment.
Do you think it's a good idea to explicitly raise an error for workspace = false in either location?
Right now, we silently treat workspace = false as if it were workspace = true, which seems problematic. cargo might enforce an error in the [lints] table, but everything in the metadata table is up to us.
There was a problem hiding this comment.
Right now, we silently treat
workspace = falseas if it wereworkspace = true
We treat it, at least in the [metadata] table, as the same as if the key was not set. I don't think cargo_toml distinguishes a value of false from None, but we could deserialize the metadata workspace as an Option<bool>, then raise an error if we get Some(false). Does that sound like a good idea?
There was a problem hiding this comment.
Ah, I was confused by the variable naming -- I thought metadata_workspace_key was testing for the presence of the key, but now I see it's actually holding the value instead.
Your call in that case, I trust your judgment.
Now workspace overrides only apply when either the
lints.workspace = trueorpackage.metadata.cargo-semver-checks.lints.workspace = truekey is set, matching the cargo[lints]table behavior. Also adds a test to ensure this behavior stays this way.