-
Notifications
You must be signed in to change notification settings - Fork 208
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
Add ImageMap #419
Add ImageMap #419
Conversation
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. Only a few questions/thoughts; no requested changes per se.
@@ -233,3 +233,46 @@ func TestPorter_paramRequired(t *testing.T) { | |||
require.True(t, ok) | |||
require.True(t, p2.Required) | |||
} | |||
|
|||
func TestPorter_generateImages(t *testing.T) { |
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.
Do we want test(s) that vet the required fields when defining an ImageMap in a porter.yaml vs fields that can be empty (omitempty
)? Or would this more be testing general yaml -> golang struct flow and thus perhaps not needed?
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.
Sorry, I'm not quite sure what you want to test here? Do you mean like the kind of test that I did for cnab-go here? To ensure that the proper set of fields have omit empty and force people to think about it each time a field is added?
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 yup, something similar to what you've done in the link provided. Can be a follow-up if we deem it necessary.
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.
Yeah I can do that in a follow-up PR since it's a bit larger in scope than just image map, and we can adjust which ones do have omit empty on them at the same time.
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 am not sure I'm going to get to this today so here's an issue to track it #425
This adds manual support for image map. The data can be defined in the porter.yaml and porter will populate it in the generated bundle.json file, so that it is made available at runtime per the spec.
However porter and the mixins do not have any explicit behaviors based on the presence of the data. When a user populates the image map, they must also add scripts to the bundle to use it at runtime.