Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ generate-docs: ## Update/generate the docs
./hack/docs/generate.sh

.PHONY: generate-charts
generate-charts: ## Re-generate the helm chart testdata only
generate-charts: build ## Re-generate the helm chart testdata only
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we need to call here make install instead right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I never actually use make install, I find it too intrusive.
I prefer to use the binary in the bin folder.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. So, based on your POV, we might need to leave it as it is.
We recommend using Make Install in all places.

However, it has contributors do not like it, they will prefer as you to build
So, leaving it as it is allows people to have a choice

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, it makes sense to use the kubebuilder version built from main since this is regenerating data in the ./testdata directory.
The make generate command, and more specifically, the make generate-testdata command, calls the script in https://github.com/kubernetes-sigs/kubebuilder/blob/master/test/testdata/generate.sh#L123C1-L123C9 which is also building kubebuilder (https://github.com/kubernetes-sigs/kubebuilder/blob/master/test/common.sh#L108-L113) and putting it in a tmp directory instead of relying on the locally installed version.

But feel free to close the PR if you thing it's best to leave it as is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your argumentation is fair enough 👍

rm -rf testdata/project-v4-with-plugins/dist/chart
(cd testdata/project-v4-with-plugins && kubebuilder edit --plugins=helm/v1-alpha)
(cd testdata/project-v4-with-plugins && ../../bin/kubebuilder edit --plugins=helm/v1-alpha)

.PHONY: check-docs
check-docs: ## Run the script to ensure that the docs are updated
Expand Down
Loading