-
Notifications
You must be signed in to change notification settings - Fork 118
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
feat: add Makefile command for updating auto-generated docs #2014
Conversation
Signed-off-by: Yash Pimple <yashpimple22@gmail.com>
✅ Deploy Preview for keptn-lifecycle-toolkit ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Kudos, SonarCloud Quality Gate passed! |
@@ -46,3 +46,34 @@ markdownlint: | |||
|
|||
markdownlint-fix: | |||
$(MDL_CMD) --entrypoint="markdownlint-cli2-fix" davidanson/markdownlint-cli2-rules:${MDL_DOCKER_VERSION} "**/*.md" | |||
|
|||
# Define the directories |
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 suppose these changes are to be made in metrics-operator/Makefile
and lifecycle-operator/Makefile
. It doesn't make sense to regenerate the resources for lifecycle-operator
, if the API has been changed for only the metrics-operator
.
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'll focus on updating the metrics-operator/Makefile
accordingly to target the specific API changes
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 put all the code here in the root Makefile
. This is not only about docs so I think that's the more fitting place.
|
||
generate-manifests: | ||
@echo "Generating manifests..." | ||
cd $(OPERATOR_DIR) && make generate && make manifests |
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.
please use make -C $(OPERATOR_DIR)
to call make command from sub-makefiles. same for metrics operator
|
||
copy-manifests: | ||
@echo "Copying manifests to Helm..." | ||
cp $(CONFIG_DIR)/crd/bases/*.yaml $(HELM_TEMPLATES_DIR)/ |
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.
you will need to use kustomize to generate the crds here. just copying the yaml files will not be enough since there are some patch files that are applied to the CRD yamls by kustomize
awk '/metadata:/{print;print " labels:\n app.kubernetes.io/part-of: keptn-lifecycle-toolkit\n crdGroup: metrics.keptn.sh\n keptn.sh/inject-cert: \"true\"";next}1' $$file > $$tmpfile; \ | ||
awk '/metadata:/{print;print " annotations:\n cert-manager.io/inject-ca-from: \"{{ .Release.Namespace }}/klt-certs\"";next}1' $$tmpfile > $$file; \ |
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 doing this in a different way. is there a way to patch the yaml with kustomize for example?
we did that before here: https://github.com/keptn/lifecycle-toolkit/blob/v0.7.1/helm/overlay/kustomization.yaml
|
||
update-crd-docs: | ||
@echo "Updating CRD docs..." | ||
cd .github/scripts/generate-crd-docs/ && ./generate-crd-docs.sh |
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.
cd .github/scripts/generate-crd-docs/ && ./generate-crd-docs.sh | |
./.github/scripts/generate-crd-docs/generate-crd-docs.sh |
Hey @YashPimple are you working on this still? |
Hello @RealAnna I apologize for the oversight, I forgot about it. I'll promptly make a PR soon. Thank You for your patience and support! |
any updates on this @YashPimple ? :) |
This PR has been automatically marked as stale because it has not had recent activity. It will be |
Again asking for updates... @YashPimple |
Hello @mowies sorry for the late update. |
Hi @YashPimple can you please share more details about the issue you are having with this one? |
Hi @YashPimple any updates on this PR? |
Closing this PR due to inactivity as the issue is already assigned to another contributor. Please feel free to look and take other tickets if you are interested! |
Fix #1879
Changes made:
resources
.