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

Default ?err to (missing) required VAR name. Fixed #6587. #6588

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@javabrett
Copy link

javabrett commented Mar 17, 2019

Changes the default (no custom ?err) error-message for a missing required variable ${MYVAR:?} to end with : MYVAR e.g.

ERROR: Missing mandatory value for "environment" option in service "myservice": MYVAR

Behaviour with ${MYVAR:?err} remains unchanged.

Fixed #6587.

Default ?err to (missing) required VAR name. Fixed #6587.
Signed-off-by: Brett Randall <javabrett@gmail.com>
@jcsirot

This comment has been minimized.

Copy link

jcsirot commented Mar 20, 2019

Thank you for this PR.
I'm wondering if we would not prefer always displaying the variable name in the error message along with the additional error message (independently of the fact whether it is empty or not).
What do you think @ulyssessouza ?

@javabrett

This comment has been minimized.

Copy link
Author

javabrett commented Apr 2, 2019

Is there a preference developing for just forcing the variable into the message/template? Need to update the PR?

Preferred format (with no err)?:

ERROR: Missing mandatory value for "environment" variable "MYVAR" in service "myservice":

That now has everything but the key e.g. MYENV in #6587.

@jcsirot @ulyssessouza

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.