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

Add more explanatory error message when not copying license files #221

Conversation

matthiasbeyer
Copy link
Contributor

Closes #207

Does the error message look good to you?

Signed-off-by: Matthias Beyer <kontakt@beyermatthias.de>
@wolfv
Copy link
Member

wolfv commented Oct 20, 2023

Definitely an improvement. I think we were considering to do some more "book-keeping" and make sure that each include glob matches at least one file. So that you get a warning (or error) if an include glob doesn't match any file.

What do you think about that?

@matthiasbeyer
Copy link
Contributor Author

I think we can definitively do that, but for that I would refactor the copy_dir function into something a bit more powerful. I would think about an object that the caller crafts (builder pattern style) and then runs, returning some state object as result. API wise I would think of something like that:

let copy = CopyDir::new()
    .at_path(path)
    .with_include_glob(foo)
    .with_include_glob(bar) // or a fn that takes multiple of these
    .with_exclude_glob(baz)
    .build()?;
    
let res = copy.run()?;
let includes_matched = res.include_globs.iter().any(|ig| ig.matched());

and possibly some more functionality. This way we do not have to fiddle with returning tuples and possibly ambigious bool flags in the return value.

What do you think?

@baszalmstra
Copy link
Contributor

I thinks thats a great idea!

@matthiasbeyer
Copy link
Contributor Author

Ok, than that's what I'll do next.

Feel free to merge this as-is though, so we have the nice(r) error message in asap! 😆

@wolfv wolfv merged commit e006be3 into prefix-dev:main Oct 20, 2023
5 checks passed
@matthiasbeyer matthiasbeyer deleted the more-explanatory-error-message-for-not-copying-license-files branch October 31, 2023 08:19
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.

Improve warning / change to error when glob not matching anything
3 participants