Skip to content
This repository has been archived by the owner on Jun 13, 2021. It is now read-only.

Validate parameters against the actual compose file #710

Merged
merged 2 commits into from
Oct 29, 2019

Conversation

ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Oct 24, 2019

- What I did
add a validation step to ensure parameters are actually used by compose file

- How I did it
Reused ExtractVariables from init and compare with parameters

- How to verify it
validate a dockerapp with unecessart parameters

- Description for the changelog
docker app validate do check parameters are in actual use by compose file

- A picture of a cute animal (not mandatory but encouraged)
image

Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

Looks legit 😄

@ndeloof ndeloof force-pushed the APP-117 branch 2 times, most recently from 064895e to 507e365 Compare October 24, 2019 12:15
@codecov
Copy link

codecov bot commented Oct 24, 2019

Codecov Report

Merging #710 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #710   +/-   ##
======================================
  Coverage    71.2%   71.2%           
======================================
  Files          59      59           
  Lines        3011    3011           
======================================
  Hits         2144    2144           
  Misses        583     583           
  Partials      284     284

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e215e7...5fe2739. Read the comment docs.

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
"testing"

"gotest.tools/assert"

Copy link
Contributor

Choose a reason for hiding this comment

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

import order looks wrong 🤣

Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM

@eunomie eunomie merged commit 0631b74 into docker:master Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants