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.

Sign up for GitHub

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

Jump to bottom

feat: support labels key in transformer configuration #5556

Merged
merged 3 commits into from
Apr 25, 2024
Merged

feat: support labels key in transformer configuration #5556

merged 3 commits into from
Apr 25, 2024

Conversation

stormqueen1990
Copy link
Member

@stormqueen1990 stormqueen1990 commented Feb 28, 2024 •
edited
Loading

Allow the usage of a separate transformer configuration for the labels key, similar to what is currently available for commonLabels and commonAnnotations. This aims to provide the same functionality that commonLabels currently provide for labels, since commonLabels is deprecated and slated for removal in a future release.

This implementation is partially based the work done in PR #5086

Fixes #4816

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 28, 2024
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 28, 2024
@antoooks
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 27, 2024
@stormqueen1990
Copy link
Member Author

/remove-lgtm

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 27, 2024
@stormqueen1990 stormqueen1990 force-pushed the feat/add-labels-transformer-config branch from 0ee7f36 to ce7a2ab Compare April 7, 2024 22:47
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 7, 2024
@stormqueen1990 stormqueen1990 marked this pull request as ready for review April 7, 2024 22:49
@k8s-ci-robot k8s-ci-robot requested a review from koba1t April 7, 2024 22:49
@stormqueen1990
Copy link
Member Author

/unassign @antoooks
/cc @koba1t

@stormqueen1990 stormqueen1990 force-pushed the feat/add-labels-transformer-config branch from ce7a2ab to 7109804 Compare April 7, 2024 23:01
@stormqueen1990 stormqueen1990 changed the title [WIP] feat: support labels key in transformer configuration feat: support labels key in transformer configuration Apr 7, 2024
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 7, 2024
@stormqueen1990 stormqueen1990 force-pushed the feat/add-labels-transformer-config branch 2 times, most recently from 0d48763 to d340e96 Compare April 8, 2024 02:58
@k8s-ci-robot
Copy link
Contributor

This PR has multiple commits, and the default merge method is: merge.
You can request commits to be squashed using the label: tide/merge-method-squash

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/test-infra repository.

@koba1t
Copy link
Member

koba1t commented Apr 17, 2024

/assign koba1t

@koba1t
Copy link
Member

koba1t commented Apr 18, 2024

Hi @stormqueen1990
I think this PR looks good!
I added a few minor comments. Could you check those and fix the conflict?

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 19, 2024
@stormqueen1990
feat: support labels key in transformer configuration
e8478ae 
Allow the usage of a separate transformer configuration for the labels key,
similar to what is currently available for commonLabels and commonAnnotations.
This aims to provide the same functionality that commonLabels currently provide
for labels, since commonLabels is deprecated and slated for removal in a future
release.
@stormqueen1990
chore(transformerconfig): add nolint hint
a5eefbb 
Add a nolint hint to the new method so the returns can stay consistent with
one another.
@stormqueen1990
fix: changes from code review
5ba4be3 
* Rename methods `AddCommonLabelFieldSpec` and `AddLabelFieldSpec` to
  `AddCommonLabelsFieldSpec` and `AddLabelsFieldSpec`.
* Add extra test to verify scenarios applying labels to Custom Resource Definitions.
@stormqueen1990 stormqueen1990 force-pushed the feat/add-labels-transformer-config branch from be38001 to 5ba4be3 Compare April 19, 2024 02:56
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 19, 2024
@stormqueen1990
Copy link
Member Author

Hi @stormqueen1990 I think this PR looks good! I added a few minor comments. Could you check those and fix the conflict?

Done! Let me know if there's anything else that needs to be addressed 😄

@koba1t
Copy link
Member

koba1t commented Apr 25, 2024

/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Apr 25, 2024
@koba1t
Copy link
Member

koba1t commented Apr 25, 2024

Thanks for your work!
/approve

@koba1t
Copy link
Member

koba1t commented Apr 25, 2024

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 25, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: koba1t, stormqueen1990

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 Apr 25, 2024
@k8s-ci-robot k8s-ci-robot merged commit 671de16 into kubernetes-sigs:master Apr 25, 2024
10 checks passed
@stormqueen1990 stormqueen1990 mentioned this pull request Apr 28, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@koba1t koba1t koba1t approved these changes

@ncapps ncapps Awaiting requested review from ncapps

@varshaprasad96 varshaprasad96 Awaiting requested review from varshaprasad96

Assignees

@koba1t koba1t

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/XL Denotes a PR that changes 500-999 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for extra fieldspecs in configurations for Labels
4 participants
@stormqueen1990 @k8s-ci-robot @antoooks @koba1t