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
Generalize manifest copy #209
Generalize manifest copy #209
Conversation
Allow arbitrary files to be copied from the manifest directory, not just those that have the .yaml file extension.
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.
Nice!
I made note of a few things I think we should follow up on too.
/lgtm
fileContent += fmt.Sprintf("COPY %s %s\n", relativeManifestDirectory, "/manifests/") | ||
fileContent += fmt.Sprintf("COPY %s %s%s\n", filepath.Join(relativeMetadataDirectory, AnnotationsFile), "/metadata/", AnnotationsFile) |
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.
Potential Followup:
I think we want the destination directories for manifests and metadata to be parameterized and fed the same arguments as the labels above, for consistency.
@@ -208,7 +208,7 @@ func TestGenerateDockerfileFunc(t *testing.T) { | |||
"LABEL operators.operatorframework.io.bundle.package.v1=test4\n"+ | |||
"LABEL operators.operatorframework.io.bundle.channels.v1=test5\n"+ | |||
"LABEL operators.operatorframework.io.bundle.channel.default.v1=test5\n\n"+ | |||
"COPY test2/*.yaml /manifests/\n"+ | |||
"COPY test2 /manifests/\n"+ | |||
"COPY metadata/annotations.yaml /metadata/annotations.yaml\n", MetadataDir) |
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.
nit: I think this should be formatted so that the metadata directory always matches the label.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kevinrizza, njhale The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Allow arbitrary files to be copied from the manifest directory, not just
those that have the .yaml file extension.
Description of the change:
Motivation for the change:
Reviewer Checklist
/docs