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

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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