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 a better error message for missing RootRules #6712

Merged
merged 3 commits into from Nov 5, 2018

Conversation

Projects
None yet
2 participants
@blorente
Contributor

blorente commented Nov 1, 2018

Problem

See #6677 for a description of the problem.

Solution

Implemented a new error message for that special case, and added a unit test to cover it.

Fixes #6677

@stuhood

stuhood approved these changes Nov 1, 2018

Thanks!

if params.len() > 1 { "s" } else { "" },
params_str(&params),
),
reason: if params_str(&params) == "()" {

This comment has been minimized.

@stuhood

stuhood Nov 1, 2018

Member

Rather than looking at the string value here, you should be able to check params.is_empty().

),
reason: if params_str(&params) == "()" {
format!(
"No root rule found to compute {}. Maybe declare it as a RootRule({}).",

This comment has been minimized.

@stuhood

stuhood Nov 1, 2018

Member

I think that the shape of this error message should align with the other case, because in this case we still failed to find any rules to satisfy the input. So maybe:

No rule found to compute {}. Maybe declare it as a RootRule({})?

Also, it would be good to align the capitalization in these error messages, but we can do that in a followup change.

@blorente blorente force-pushed the blorente:blorente/6677/better-rootrule-errors branch from 30f8633 to 00bd21a Nov 1, 2018

@stuhood

stuhood approved these changes Nov 5, 2018

@stuhood stuhood merged commit 6a9f4f8 into pantsbuild:master Nov 5, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment