Skip to content
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

[genref] Strip prefix in map types. #343

Merged
merged 1 commit into from
Oct 27, 2023

Conversation

trasc
Copy link
Contributor

@trasc trasc commented Oct 26, 2023

This PR applies the StripPrefix logic for map types (keys and values).

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 26, 2023
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 26, 2023
@tengqm
Copy link
Contributor

tengqm commented Oct 26, 2023

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tengqm, trasc

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 26, 2023
@alculquicondor
Copy link

@tengqm did you miss the LGTM or are you waiting for someone else to take a look?

@sftim
Copy link
Contributor

sftim commented Oct 26, 2023

This needs an LGTM review in order to merge. @alculquicondor if you can point this PR out to people who review Go code, it'll help.

@sftim
Copy link
Contributor

sftim commented Oct 26, 2023

(I don't know your intent here @tengqm )

@tengqm
Copy link
Contributor

tengqm commented Oct 26, 2023

I was trying to let someone else chime in with a lgtm, although I think this is a good improvement to the current code.
If the PR receives no reviews from other reviewers, and I still remember here is a PR to be merged, I'll give it a lgtm.

@trasc
Copy link
Contributor Author

trasc commented Oct 27, 2023

@tengqm @sftim Not necessary related to this PR, but , is there a plan to have releases in this project, or adding go module version tags?

@tengqm
Copy link
Contributor

tengqm commented Oct 27, 2023

@tengqm @sftim Not necessary related to this PR, but , is there a plan to have releases in this project, or adding go module version tags?

No plan yet. Open to all kinds of suggestions.

@trasc
Copy link
Contributor Author

trasc commented Oct 27, 2023

@tengqm @sftim Not necessary related to this PR, but , is there a plan to have releases in this project, or adding go module version tags?

No plan yet. Open to all kinds of suggestions.

We are thinking of using genref in kueue (https://kueue.sigs.k8s.io/docs/) and being able to reference by a version instead of a commit hash will help.

Currently we are doing something like:

https://github.com/kubernetes-sigs/kueue/blob/099ed21f62fe46f2ee451f7ffa40bcea03def9ff/Makefile#L317-L321

@tengqm
Copy link
Contributor

tengqm commented Oct 27, 2023

@tengqm @sftim Not necessary related to this PR, but , is there a plan to have releases in this project, or adding go module version tags?

No plan yet. Open to all kinds of suggestions.

We are thinking of using genref in kueue (https://kueue.sigs.k8s.io/docs/) and being able to reference by a version instead of a commit hash will help.

Currently we are doing something like:

https://github.com/kubernetes-sigs/kueue/blob/099ed21f62fe46f2ee451f7ffa40bcea03def9ff/Makefile#L317-L321

Okay. If a version tag is needed, we can actually do it starting now.
How about we tag the current master head as 'v0.28.0' or something like that?

@trasc
Copy link
Contributor Author

trasc commented Oct 27, 2023

@tengqm @sftim Not necessary related to this PR, but , is there a plan to have releases in this project, or adding go module version tags?

No plan yet. Open to all kinds of suggestions.

We are thinking of using genref in kueue (https://kueue.sigs.k8s.io/docs/) and being able to reference by a version instead of a commit hash will help.
Currently we are doing something like:
https://github.com/kubernetes-sigs/kueue/blob/099ed21f62fe46f2ee451f7ffa40bcea03def9ff/Makefile#L317-L321

Okay. If a version tag is needed, we can actually do it starting now. How about we tag the current master head as 'v0.28.0' or something like that?

It should be fine, but in order to reference genref we might also need the genref/v0.28.0 tag, and it will be even better if this PR is also merged.

@tengqm
Copy link
Contributor

tengqm commented Oct 27, 2023

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 27, 2023
@tengqm tengqm merged commit 23a78f4 into kubernetes-sigs:master Oct 27, 2023
2 checks passed
@tengqm
Copy link
Contributor

tengqm commented Oct 27, 2023

PR kicked in and tag 'v0.28.0' for the project has been added.

@trasc trasc deleted the strip-prefix-map branch October 30, 2023 06:03
@trasc
Copy link
Contributor Author

trasc commented Oct 30, 2023

PR kicked in and tag 'v0.28.0' for the project has been added.

Thanks @tengqm.
Can you also push a tag called genref/v0.28.0?
We need that to be able to do thinks like go install github.com/kubernetes-sigs/reference-docs/genref@0.28.0 since genref it's a go module (has it's own go.mod/ go.sum).

@tengqm
Copy link
Contributor

tengqm commented Oct 30, 2023

PR kicked in and tag 'v0.28.0' for the project has been added.

Thanks @tengqm. Can you also push a tag called genref/v0.28.0? We need that to be able to do thinks like go install github.com/kubernetes-sigs/reference-docs/genref@0.28.0 since genref it's a go module (has it's own go.mod/ go.sum).

done

@trasc
Copy link
Contributor Author

trasc commented Oct 30, 2023

PR kicked in and tag 'v0.28.0' for the project has been added.

Thanks @tengqm. Can you also push a tag called genref/v0.28.0? We need that to be able to do thinks like go install github.com/kubernetes-sigs/reference-docs/genref@0.28.0 since genref it's a go module (has it's own go.mod/ go.sum).

done

Hi @tengqm, it looks to be a small issue , the tag should be genref/v0.28.0. (not generef/v0.28.0)

@tengqm
Copy link
Contributor

tengqm commented Oct 31, 2023

should be okay now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants