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

Complain to stderr about unused raze settings. #18

Merged
merged 2 commits into from
Mar 12, 2018

Conversation

mfarrugi
Copy link
Collaborator

Found myself upgrading crates and not (quickly) noticing that overrides were no longer being applied.

Not sure if this should be a harder error, lmk if you'd like it done differently. The output is kind of noisy right now as well, which may make this hard to notice if you don't know to look for it.

@acmcarther
Copy link
Member

This looks good to me. At some point I planned to print something like
Found X @ t1, which didn't have a matching dependency, did you mean X @ [t2,t3,t4]
and I think there is code like that floating around somewhere in some other repo..

If you want to change this to hint at the available other versions, that would be nice, else we can do it some time later. LMK either way.

@mfarrugi
Copy link
Collaborator Author

@acmcarther I added the version hint

@acmcarther
Copy link
Member

LGTM, thanks for quick fix!

@mfarrugi
Copy link
Collaborator Author

@acmcarther I want to promote this to a hard error (since I ran into it again :|), any objection?

The other option would be to replace the Generated $CRATE successfully with Generated $count crates successfully, but I imagine most people are going to ignore non-error output that takes more than 1 screen.

@acmcarther
Copy link
Member

Changing the warning to an error SGTM

Code was restructured recently, here is where the warning happens now:

pub fn warn_unused_settings(
all_crate_settings: &HashMap<String, CrateSettingsPerVersion>,
all_packages: &Vec<Package>,
) {
let mut known_versions_per_crate = HashMap::new();
for &Package {
ref name,
ref version,
..
} in all_packages.iter()
{
known_versions_per_crate
.entry(name.clone())
.or_insert(HashSet::new())
.insert(version.clone());
}
for (name, settings_per_version) in all_crate_settings.iter() {
if !known_versions_per_crate.contains_key(name) {
eprintln!(
"Found unused raze settings for all of {}-{:?}",
name,
settings_per_version.keys()
);
// No version introspection needed -- no known version of this crate
continue;
}
// UNWRAP: Guarded above
let all_known_versions = known_versions_per_crate.get(name).unwrap();
for version in settings_per_version.keys() {
if !all_known_versions.contains(version) {
eprintln!(
"Found unused raze settings for {}-{}, but {:?} were known",
name, version, all_known_versions
)
}
}
}
}

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

Successfully merging this pull request may close these issues.

2 participants