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

Move the bless command to a field in Config #175

Merged
merged 1 commit into from
Nov 8, 2023

Conversation

Alexendoo
Copy link
Contributor

The usage in clippy showed that #157 made the API a bit awkward with regards to the bless command and conflict handling modes, this takes it from

let mut config = Config {
    ..
};
config.with_args(&args, /* bless by default */ false);
if let OutputConflictHandling::Error(err) = &mut config.output_conflict_handling {
    *err = "cargo uibless".into();
}

to

let mut config = Config {
    output_conflict_handling: OutputConflictHandling::Error,
    bless_command: Some("cargo uibless".into()),
    ..
};
config.with_args(&args)

The default conflict handling is now only specified in the Config, no more bool in with_args

It's a separate (optional) field because in order for OutputConflictHandling::Bless + --check to work it had to supply a bless command of its own, but there's not really a sensible default suggestion we can make

@oli-obk oli-obk force-pushed the bless-command-config branch from a811736 to 3350bb6 Compare November 8, 2023 10:56
@oli-obk oli-obk merged commit c239bb8 into oli-obk:main Nov 8, 2023
5 checks passed
@Alexendoo Alexendoo deleted the bless-command-config branch November 8, 2023 13:43
bors added a commit to rust-lang/rust-clippy that referenced this pull request Feb 19, 2024
Default test output conflict handling to error

oli-obk/ui_test#175 got rid of the `bool` that controlled the default handling so we need to specify it ourselves

r? `@flip1995`

changelog: none
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