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

prefer list format for extra_hosts to preserve backward compatibility #326

Merged
merged 1 commit into from Dec 7, 2022

Conversation

ndeloof
Copy link
Collaborator

@ndeloof ndeloof commented Dec 6, 2022

While map format is supported by Docker Compose v1, and proposed to be officially documented (compose-spec/compose-spec#285), still most users rely on the list format and we should prefer this one to enforce backward compatibility.

close docker/compose#10038

@ndeloof ndeloof requested review from milas and glours December 6, 2022 12:09
types/types.go Outdated
Comment on lines 357 to 362
// is specified as a struct tag. Explicitly empty commands (i.e. `[]` or ``)
// will serialize to an empty array (`[]`).
//
// When marshaled to JSON, the `omitempty` struct must NOT be specified.
// If the command field is nil, it will be serialized as `null`.
// Explicitly empty commands (i.e. `[]` or `''`) will serialize to an empty
// Explicitly empty commands (i.e. `[]` or ``) will serialize to an empty
Copy link
Member

Choose a reason for hiding this comment

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

A wild smart quote has appeared! s/”/''/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🤨 Wonder where this came from. Don't even know how to type this character. I assume my IDE tried to be clever ?

@@ -539,6 +539,14 @@ func (h HostsList) AsList() []string {
return l
Copy link
Member

Choose a reason for hiding this comment

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

Add a sort.Strings to ensure stable output?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nice suggestion!

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Copy link
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

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

LGTM

@ndeloof ndeloof merged commit 498a1aa into compose-spec:master Dec 7, 2022
@ndeloof ndeloof deleted the extra_hosts branch December 7, 2022 07:26
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.

[BUG] cli multifile "config" generates bad extra_hosts entries
3 participants