Skip to content

mark node phase as deprecated #549

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

Merged
merged 1 commit into from
Sep 26, 2016
Merged

Conversation

mwhooker
Copy link
Contributor

@mwhooker mwhooker commented May 20, 2016

I'm not sure on what the best wording for this should be, but Node Phase is no longer populated, and so the docs should say so.

@Random-Liu on slack/#sig-node told me that they believed node phase was unused.

cc @dawnchen


This change is Reviewable

I'm not sure on what the best wording for this should be, but `Node Phase` is no longer populated, and so the docs should say so.

@Random-Liu on slack/#sig-node told me that they believed node phase was unused.

cc @dawnchen
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@mwhooker
Copy link
Contributor Author

signed CLA

@googlebot
Copy link

CLAs look good, thanks!

@Random-Liu
Copy link
Member

I'm sure the NodePhase is not used in the code, but I'm not sure whether we have the plan to deprecate it or not.

@dchen1107 and @bgrant0607 should have more ideas.

@bgrant0607
Copy link
Member

@dchen1107 Did we change how/whether node phase is set? That would be a breaking behavioral change for the v1 API.

If we do plan to deprecate it, it would need to be documented in the API first:
https://github.com/kubernetes/kubernetes/blob/master/pkg/api/v1/types.go#L2327

@pwittrock
Copy link
Member

@dchen1107 @Random-Liu Any update on this?

@Random-Liu
Copy link
Member

Random-Liu commented Jul 18, 2016

@pwittrock I'm not sure whether we really want to deprecate NodePhase, have to confirm with @dchen1107. She will come back from holiday next Monday. :)

@pwittrock
Copy link
Member

@dchen1107 Is NodePhase still populated? What should the docs say?

@dchen1107
Copy link
Member

We stopped populate / set node phase a while back. This won't be breaking change for v1 API since no one couldn't depend on it at all. But we still should change v1API code first to make such announcement. Let me send a pr changing v1API shortly.

@pwittrock
Copy link
Member

pwittrock commented Aug 11, 2016

@mwhooker This field will be deprecated in kubernetes/kubernetes#30005 for 1.4. Lets merge it when that change is live.

@mwhooker
Copy link
Contributor Author

neato! thanks

k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this pull request Aug 12, 2016
Automatic merge from submit-queue

Marked NodePhase deprecated.

cc/ @lavalamp 

Even Kubernetes stops set NodePhase for a while, the user might patch NodeStatus with NodePhase. Marked the field deprecated and documented first. 

xref: kubernetes/website#549

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.kubernetes.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.kubernetes.io/reviews/kubernetes/kubernetes/30005)
<!-- Reviewable:end -->
@devin-donnelly devin-donnelly added this to the 1.4 milestone Sep 9, 2016
@devin-donnelly
Copy link
Contributor

This docs change is slated for the 1.4 release. We're trying to have all 1.4 docs PRs target the release-1.4 branch. Can you have this PR target release-1.4? A rebase would probably also be a good idea.

@devin-donnelly devin-donnelly added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. and removed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Sep 26, 2016
@devin-donnelly devin-donnelly merged commit 971f6a4 into kubernetes:master Sep 26, 2016
@mwhooker mwhooker deleted the patch-1 branch October 17, 2017 05:29
mikutas pushed a commit to mikutas/k8s-website that referenced this pull request Feb 24, 2021
Okabe-Junya pushed a commit to Okabe-Junya/website that referenced this pull request Dec 4, 2023
Bumps [actions/checkout](https://github.com/actions/checkout) from 2 to 3.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v2...v3)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants