Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Deprecated binding for 1.7 #47041

Merged
merged 3 commits into from
Jun 8, 2017
Merged

Deprecated binding for 1.7 #47041

merged 3 commits into from
Jun 8, 2017

Conversation

k82cn
Copy link
Member

@k82cn k82cn commented Jun 6, 2017

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #10043

Deprecated Binding objects in 1.7.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 6, 2017
@k8s-github-robot k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jun 6, 2017
@k82cn
Copy link
Member Author

k82cn commented Jun 6, 2017

/cc @smarterclayton

@k82cn
Copy link
Member Author

k82cn commented Jun 6, 2017

hi team, would you help to review it? I'd like to get it merge in 1.7 :).

gyliu513
gyliu513 reviewed Jun 6, 2017
View reviewed changes
pkg/api/types.go Outdated
@@ -3135,7 +3135,8 @@ type NamespaceList struct {
Items []Namespace
}

// Binding ties one object to another - for example, a pod is bound to a node by a scheduler.
// Binding ties one object to another; for example, a pod is bound to a node by a scheduler.
// Deprecated in 1.7.
Copy link
Contributor

Choose a reason for hiding this comment

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

How about move Deprecated in 1.7. before the comments?

Such as

// Deprecated in 1.7.
// Binding ties one object to another; for example, a pod is bound to a node by a scheduler.

Copy link
Member Author

Choose a reason for hiding this comment

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

seems both style are there in the doc :).

Copy link
Contributor

Choose a reason for hiding this comment

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

Never mind, depend on you.

pkg/api/v1/types.go Outdated
// Binding ties one object to another.
// For example, a pod is bound to a node by a scheduler.
// Binding ties one object to another; for example, a pod is bound to a node by a scheduler.
// Deprecated in 1.7.
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 6, 2017
@smarterclayton
Copy link
Contributor

@kubernetes/sig-api-machinery-pr-reviews

@k8s-ci-robot k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jun 6, 2017
@k82cn
Copy link
Member Author

k82cn commented Jun 7, 2017

rebased :).

@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 7, 2017
@k8s-github-robot k8s-github-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 7, 2017
@sttts
Copy link
Contributor

sttts commented Jun 7, 2017

Protobuf is broken. Otherwise lgtm.

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 7, 2017
@k82cn
Copy link
Member Author

k82cn commented Jun 7, 2017

Green now :)

@ sttts , @deads2k can you help to approve? It's better to deprecate it as early as possible :).

@sttts
Copy link
Contributor

sttts commented Jun 7, 2017

@smarterclayton approved?

@timothysc timothysc added this to the v1.7 milestone Jun 7, 2017
@timothysc timothysc added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Jun 7, 2017
@k82cn
Copy link
Member Author

k82cn commented Jun 8, 2017

@smarterclayton , would you help to approve this? This PR deprecated "Binding" (/binding) as we discuss. It's better to deprecate in this release (1.7) :).

@smarterclayton
Copy link
Contributor

/approve

Starts the clock on removing the old resource

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 8, 2017
@smarterclayton
Copy link
Contributor

No release risk, communicates intent.

@smarterclayton
Copy link
Contributor

Actually, one tweak? Can you say expand the text to say "deprecated in 1.7, please use the bindings subresource of pods instead."?

@k82cn
Copy link
Member Author

k82cn commented Jun 8, 2017

sure, np; updates it right now :).

@k82cn
Copy link
Member Author

k82cn commented Jun 8, 2017

Done, ping for lgtm :).

@sttts
Copy link
Contributor

sttts commented Jun 8, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 8, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: k82cn, smarterclayton, sttts

Associated issue: 10043

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 5404948 into kubernetes:master Jun 8, 2017
@k82cn k82cn deleted the k8s_10043 branch June 8, 2017 06:13
@heyste heyste mentioned this pull request Sep 13, 2022
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@gyliu513 gyliu513 gyliu513 left review comments

@smarterclayton smarterclayton Awaiting requested review from smarterclayton

Assignees

@sttts sttts

@smarterclayton smarterclayton

@deads2k deads2k

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. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.7
Development

Successfully merging this pull request may close these issues.

Delete /binding
8 participants
@k82cn @smarterclayton @sttts @k8s-github-robot @gyliu513 @timothysc @deads2k @k8s-ci-robot