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

✨ Allow setting controller name, fix default #458

Merged
merged 1 commit into from
May 30, 2019
Merged

✨ Allow setting controller name, fix default #458

merged 1 commit into from
May 30, 2019

Conversation

DirectXMan12
Copy link
Contributor

This switches the default controller name in the builder to just
lower() which fixes any issues that arrise from it showing up in
the deprecated metrics (which use the workqueue name as part of the
metric name).

It also allows overriding name in case of two controllers reconciling
the same type using the builder.

@DirectXMan12
Allow setting controller name, fix default
2a14111 
This switches the default controller name in the builder to just
lower(<kind>) which fixes any issues that arrise from it showing up in
the deprecated metrics (which use the workqueue name as part of the
metric name).

It also allows overriding name in case of two controllers reconciling
the same type using the builder.
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 30, 2019
@DirectXMan12
Copy link
Contributor Author

cc @pwittrock

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: DirectXMan12

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 approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 30, 2019
mengqiy
mengqiy approved these changes May 30, 2019
View reviewed changes
Copy link
Member

@mengqiy mengqiy left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 30, 2019
@k8s-ci-robot k8s-ci-robot merged commit b56d269 into kubernetes-sigs:master May 30, 2019
@DirectXMan12 DirectXMan12 deleted the bug/metrics-compatible-default-name branch May 30, 2019 22:58
@pwittrock
Copy link
Contributor

woot

@DirectXMan12 DirectXMan12 mentioned this pull request Jun 3, 2019
Closed
DirectXMan12 pushed a commit that referenced this pull request Jan 31, 2020
@k8s-ci-robot
Merge pull request #458 from willise/master
11875cb 
Add more example code of book
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mengqiy mengqiy mengqiy approved these changes

@droot droot Awaiting requested review from droot

@pwittrock pwittrock Awaiting requested review from pwittrock

Assignees

@mengqiy mengqiy

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@DirectXMan12 @k8s-ci-robot @pwittrock @mengqiy