✨ (Only valid for those who consume Kubebuilder as a lib) - Allow usage of custom marker names #3993

beatrausch · 2024-06-28T04:53:11Z

This PR adds a code implementation which allow Kubebuilder lib consumes use custom names for the markers
Closes: #3990

k8s-ci-robot · 2024-06-28T04:53:19Z

Welcome @beatrausch!

It looks like this is your first PR to kubernetes-sigs/kubebuilder 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubebuilder has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2024-06-28T04:53:19Z

Hi @beatrausch. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

camilamacedo86 · 2024-06-30T19:25:11Z

pkg/machinery/marker.go

@@ -69,3 +80,17 @@ type CodeFragments []string

 // CodeFragmentsMap binds Markers and CodeFragments together
 type CodeFragmentsMap map[Marker]CodeFragments
+
+func markerPrefix(prefix string) string {
+	trimmed := strings.TrimSpace(prefix)


Will the trim revert the change:

Bug fix to ensure consistent spacing in marker annotations (#3904).

?

@camilamacedo86 Please, correct my when I am wrong, but what I understand from the issue is, that it requests a constant string representation of the marker. This is still achieved by adding a space between the comment and the prefix:

func (m Marker) String() string { return m.comment + " " + m.prefix + m.value }

Yes, you are right 👍

camilamacedo86 · 2024-06-30T19:26:08Z

pkg/machinery/marker_test.go

+	Context("String", func() {
+		DescribeTable("should return the right string representation",
+			func(marker Marker, str string) { Expect(marker.String()).To(Equal(str)) },
+			Entry("for yaml files", NewMarkerFor("test.yaml", "test"), "# +kubebuilder:scaffold:test"),


We need more one test with // right?

Sure I will add tests for //

camilamacedo86 · 2024-06-30T19:27:04Z

pkg/machinery/marker.go

@@ -33,16 +33,27 @@ var commentsByExt = map[string]string{

 // Marker represents a machine-readable comment that will be used for scaffolding purposes
 type Marker struct {
+	prefix  string
 	comment string
 	value   string
 }

 // NewMarkerFor creates a new marker customized for the specific file


I think we need to change the comment here to make clear that will create the marker with kbPrefix

Added some more details on what prefix is used

camilamacedo86 · 2024-06-30T19:27:40Z

/ok-to-test

camilamacedo86 · 2024-06-30T19:29:10Z

pkg/machinery/marker_test.go

+			Entry("for yaml files", NewMarkerWithPrefixFor("+custom:scaffold", "test.yaml", "test"), "# +custom:scaffold:test"),
+			Entry("for yaml files", NewMarkerWithPrefixFor("custom:scaffold:", "test.yaml", "test"), "# +custom:scaffold:test"),
+			Entry("for yaml files", NewMarkerWithPrefixFor("+custom:scaffold:", "test.yaml", "test"), "# +custom:scaffold:test"),
+			Entry("for yaml files", NewMarkerWithPrefixFor(" +custom:scaffold: ", "test.yaml", "test"), "# +custom:scaffold:test"),


It is not passing in the linter due the sieze of the line,
Could you please break the line?

pkg/machinery/marker_test.go:65: line is 122 characters (lll)
Entry("for yaml files", NewMarkerWithPrefixFor(" +custom:scaffold: ", "test.yaml", "test"), "#

I fixed the linter issues.

camilamacedo86

Hi @beatrausch

That is great and shows good to fly !!!
Could you please just squash all commits for we get it merged?

We have a policy to keep 1 commit per PR (unless we have a real reason to split but in this case we usually have more PRs instead of)

camilamacedo86

/lgtm

k8s-ci-robot · 2024-07-02T23:02:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: beatrausch, camilamacedo86

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:

~~OWNERS~~ [camilamacedo86]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 28, 2024

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 28, 2024

k8s-ci-robot requested review from everettraven and Kavinjsir June 28, 2024 04:53

beatrausch changed the title ~~Allow custom marker names~~ ✨ Allow custom marker names Jun 28, 2024

beatrausch changed the title ~~✨ Allow custom marker names~~ :sparkles Allow custom marker names Jun 28, 2024

beatrausch changed the title ~~:sparkles Allow custom marker names~~ ✨ Allow custom marker names Jun 28, 2024

camilamacedo86 reviewed Jun 30, 2024

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 30, 2024

camilamacedo86 reviewed Jun 30, 2024

View reviewed changes

camilamacedo86 reviewed Jul 2, 2024

View reviewed changes

extented marker with custom prefix

8d8d091

beatrausch force-pushed the marker-with-prefix branch from 291718e to 8d8d091 Compare July 2, 2024 19:31

camilamacedo86 changed the title ~~✨ Allow custom marker names~~ ✨ (Only valid for those who consume Kubebuilder as a lib) - Allow usage of custom marker names Jul 2, 2024

camilamacedo86 approved these changes Jul 2, 2024

View reviewed changes

k8s-ci-robot assigned camilamacedo86 Jul 2, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 2, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 2, 2024

k8s-ci-robot merged commit f1a4328 into kubernetes-sigs:master Jul 2, 2024
21 checks passed

beatrausch deleted the marker-with-prefix branch July 3, 2024 05:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ (Only valid for those who consume Kubebuilder as a lib) - Allow usage of custom marker names #3993

✨ (Only valid for those who consume Kubebuilder as a lib) - Allow usage of custom marker names #3993

beatrausch commented Jun 28, 2024 •

edited by camilamacedo86

Loading

k8s-ci-robot commented Jun 28, 2024

k8s-ci-robot commented Jun 28, 2024

camilamacedo86 Jun 30, 2024

beatrausch Jul 1, 2024

camilamacedo86 Jul 2, 2024

camilamacedo86 Jun 30, 2024

beatrausch Jul 1, 2024

camilamacedo86 Jun 30, 2024

beatrausch Jul 1, 2024

camilamacedo86 commented Jun 30, 2024

camilamacedo86 Jun 30, 2024

beatrausch Jul 1, 2024

camilamacedo86 left a comment

camilamacedo86 left a comment

k8s-ci-robot commented Jul 2, 2024

✨ (Only valid for those who consume Kubebuilder as a lib) - Allow usage of custom marker names #3993

✨ (Only valid for those who consume Kubebuilder as a lib) - Allow usage of custom marker names #3993

Conversation

beatrausch commented Jun 28, 2024 • edited by camilamacedo86 Loading

k8s-ci-robot commented Jun 28, 2024

k8s-ci-robot commented Jun 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camilamacedo86 commented Jun 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camilamacedo86 left a comment

Choose a reason for hiding this comment

camilamacedo86 left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jul 2, 2024

beatrausch commented Jun 28, 2024 •

edited by camilamacedo86

Loading