-
Notifications
You must be signed in to change notification settings - Fork 70
Allow ship init to be pointed to valid go-getter ship.yaml path #682
Conversation
"v1": { | ||
"config": null, | ||
"releaseName": "ship", | ||
"upstream": "file::/Users/robert/go/src/github.com/replicatedhq/ship/integration/init/local-single-file-gogetter/input/ship.yaml", |
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.
Not OK
Maybe change this to <UPSTREAM>
and include the upstream as part of the string replacement? (the map[string]string{} on line 94 of integration/init/integration_test.go)
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.
oh yea this is why i didn't push this
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.
We do variable substitution for all the init_app
tests: https://github.com/replicatedhq/ship/blob/master/integration/init_app/amazon-eks-template/expected/.ship/state.json
Seems like the idiom is __upstream__
not <UPSTREAM>
though
|
||
if isReplicatedApp { | ||
return "inline.replicated.app", finalPath, nil | ||
if isReplicatedApp { |
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.
moved this up because it checks for 2 different files, if the first is true and the second is false... 😢
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.
lgtm, but we could reduce the magic there a little
integration/init/integration_test.go
Outdated
Expect(err).NotTo(HaveOccurred()) | ||
absolutePath := filepath.Join(pwdRoot, "..") | ||
absoluteUpstream = fmt.Sprintf("file::%s", filepath.Join(absolutePath, relativePath)) | ||
replacements["__upstream__"] = absolutePath |
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 would prefer
replacements["__upstream__"] = absolutePath | |
replacements["__upstream__"] = absoluteUpstream |
"v1": { | ||
"config": null, | ||
"releaseName": "ship", | ||
"upstream": "file::__upstream__/input/ship.yaml", |
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.
and
"upstream": "file::__upstream__/input/ship.yaml", | |
"upstream": "file::__upstream__", |
What I Did
How I Did it
How to verify it
Point
ship init
to valid go-getter ship.yaml pathDescription for the Changelog
Allow ship init to be pointed to valid go-getter ship.yaml path
Picture of a Boat (not required but encouraged)
🛶