-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
2221 allow empty environment keys #2225
Conversation
It was missing a space between the different types, when there were 3 possible type values. Signed-off-by: Mazz Mosley <[email protected]>
@@ -143,7 +143,7 @@ def _parse_valid_types_from_validator(validator): | |||
if len(validator) >= 2: | |||
first_type = anglicize_validator(validator[0]) | |||
last_type = anglicize_validator(validator[-1]) | |||
types_from_validator = "{}{}".format(first_type, ", ".join(validator[1:-1])) | |||
types_from_validator = "{}".format(", ".join([first_type] + validator[1:-1])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the "{}".format()
is unnecessary now? it could be:
types_from_validator = ", ".join([first_type] + validator[:-1])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, good point. I'll update.
Environment keys that contain no value, get populated with values taken from the environment not from the build phase but from running the command `up`. Signed-off-by: Mazz Mosley <[email protected]>
e3d8ef5
to
08add66
Compare
LGTM, that test failure is a flake I see locally pretty often. I'll wait for janky to re-run before merge. |
And I opened a github issue about it a while ago: moby/moby#15995 |
Build is green after another run |
2221 allow empty environment keys
Fixes #2221