-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Fix kubernetes-manifests/kustomization.yaml empty components issue #1436
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
if [[ $file_to_copy == "./kubernetes-manifests/kustomization.yaml" ]]; then | ||
continue | ||
fi | ||
|
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 just added this change into hack/make-release-artifacts.sh
.
Context:
- Today, when a new release of Online Boutique is created
kubernetes-manifests/kustomization.yaml
is copied over tokustomize/base/kustomization.yaml
. - This should not be the case since the two files are meant to look different.
- For instance,
kustomize/base/kustomization.yaml
should not include:
# components:
or
# - loadgenerator.yaml # During development, the loadgenerator module inside skaffold.yaml will be used.
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.
@phamquiluan sorry for the delayed review.
And thank you for:
- creating a clear GitHub issue
- creating a pull-request to fix the issue
I have not reproduced the issue on my end, but I trust that your fix worked for you.
Plus, the fix won't negatively impact anyone else's user experience.
Approved!
This fixes #1435.