Skip to content

Simplify the marshaling of compose types.Config#895

Merged
thaJeztah merged 1 commit intodocker:masterfrom
vdemeester:simplify-yaml-marshal
Feb 21, 2018
Merged

Simplify the marshaling of compose types.Config#895
thaJeztah merged 1 commit intodocker:masterfrom
vdemeester:simplify-yaml-marshal

Conversation

@vdemeester
Copy link
Copy Markdown
Collaborator

  • Add Version to types.Config
  • Add a new Services types (that is just []ServiceConfig) and add
    MarshalYAML method on it.
  • Clean other top-level custom marshaling as Services is the only one
    required.

Follow-up of #891 👼

🦁

Signed-off-by: Vincent Demeester vincent@sbr.pm

- Add `Version` to `types.Config`
- Add a new `Services` types (that is just `[]ServiceConfig`) and add
  `MarshalYAML` method on it.
- Clean other top-level custom marshaling as `Services` is the only one
  required.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
@vdemeester
Copy link
Copy Markdown
Collaborator Author

The order of the marshaled Config changed because it's now using the order of the struct definition instead of natural order with the map.

@thaJeztah
Copy link
Copy Markdown
Member

Can we add some filler-code to fill up all the code that was removed? 😂

@codecov-io
Copy link
Copy Markdown

codecov-io commented Feb 21, 2018

Codecov Report

Merging #895 into master will decrease coverage by 0.04%.
The diff coverage is 33.33%.

@@            Coverage Diff            @@
##           master    #895      +/-   ##
=========================================
- Coverage   53.24%   53.2%   -0.05%     
=========================================
  Files         258     258              
  Lines       16357   16343      -14     
=========================================
- Hits         8710    8696      -14     
  Misses       7082    7082              
  Partials      565     565

Copy link
Copy Markdown
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

propertyWarnings(deprecatedProperties))
}
return config, configDetails.Version, nil
return config, nil
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

❤️

@thaJeztah thaJeztah merged commit 88ee1a6 into docker:master Feb 21, 2018
@GordonTheTurtle GordonTheTurtle added this to the 18.03.0 milestone Feb 21, 2018
@vdemeester vdemeester deleted the simplify-yaml-marshal branch February 21, 2018 18:03
nobiit pushed a commit to nobidev/docker-cli that referenced this pull request Nov 19, 2025
Simplify the marshaling of compose types.Config
Upstream-commit: 88ee1a6
Component: cli
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants