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

Emit error for all unset configuration variables #1132

Merged
merged 4 commits into from
Aug 18, 2022

Conversation

jquorning
Copy link
Contributor

Improve user experience by emitting error for each unset configuration variable.

Output of alr get lvgl_ada:

error: Configuration variable 'lvgl_ada.density_per_inch' not set and has no default value.
error: Configuration variable 'lvgl_ada.pixel_bit_depth' not set and has no default value.
error: Configuration variable 'lvgl_ada.horizontal_resolution' not set and has no default value.
error: Configuration variable 'lvgl_ada.vertical_resolution' not set and has no default value.
error: Configuration failed

Before:

error: Configuration variable 'lvgl_ada.density_per_inch not set and has no default value.

Copy link
Member

@Fabien-Chouteau Fabien-Chouteau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks @jquorning

@Fabien-Chouteau
Copy link
Member

@jquorning you might want to add a test case for that.

@mosteo
Copy link
Member

mosteo commented Aug 17, 2022

I was bitten by this very same case once. Thanks!

@mosteo
Copy link
Member

mosteo commented Aug 17, 2022

Minor last thing: I would move the test from the get to the config dir, as it applies outside of the get command.

@mosteo mosteo merged commit 74d3b96 into alire-project:master Aug 18, 2022
@mosteo
Copy link
Member

mosteo commented Aug 18, 2022

Thanks, merged.

@jquorning jquorning deleted the config-fault branch August 20, 2022 01:33
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.

3 participants