Skip to content

Conversation

@stevsmit
Copy link
Member

@stevsmit stevsmit commented Nov 23, 2021

@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 23, 2021
@stevsmit stevsmit closed this Nov 23, 2021
@netlify
Copy link

netlify bot commented Nov 23, 2021

✔️ Deploy Preview for osdocs ready!

🔨 Explore the source changes: 9933277

🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/61afd69d4068850007a8e885

😎 Browse the preview: https://deploy-preview-39184--osdocs.netlify.app

@stevsmit stevsmit reopened this Nov 23, 2021
@stevsmit stevsmit added branch/enterprise-4.9 peer-review-needed Signifies that the peer review team needs to review this PR labels Nov 23, 2021
Copy link
Contributor

@sferich888 sferich888 left a comment

Choose a reason for hiding this comment

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

+1 to moving this content; however now that I re-read the text I have a few points that we may want to try and clarify.

@stevsmit stevsmit force-pushed the BZ1900182-4-9 branch 3 times, most recently from a36329b to ded3c09 Compare November 29, 2021 22:16
Copy link
Member

Choose a reason for hiding this comment

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

These two sentences are about kubelet/API-server node skew, not oc. Can we talk about node skew somewhere outside of this oc-focused file? I'm not sure if/where we formally doc kubelet/API-server skew today, but the in-cluster logic is described here and depends on whether the OpenShift minor version is even or odd.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we document that anywhere, so if thats true we need to add it somewhere.

@stevsmit stevsmit force-pushed the BZ1900182-4-9 branch 2 times, most recently from 8f3a77a to 07701a9 Compare November 30, 2021 17:36
Copy link
Member

@wking wking left a comment

Choose a reason for hiding this comment

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

As long as the outgoing kubelet/API-server skew docs (or a successor) end up somewhere at some point (could be in a follow-up PR, doesn't have to be in this one), the rest of this PR looks good to me as it stands.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 30, 2021
@sferich888
Copy link
Contributor

/lgtm

@sdodson
Copy link
Member

sdodson commented Nov 30, 2021

/lgtm

@sjstout
Copy link

sjstout commented Dec 1, 2021

LGTM!

@lpettyjo lpettyjo changed the title Removes versioning-policy section and adds content to usage-oc module BZ1900182: Removes versioning-policy section and adds content to usage-oc module Dec 7, 2021
@lpettyjo lpettyjo added this to the Next Release milestone Dec 7, 2021
@lpettyjo lpettyjo self-requested a review December 7, 2021 18:53
Copy link
Contributor

@lpettyjo lpettyjo left a comment

Choose a reason for hiding this comment

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

Looks good!

@lpettyjo lpettyjo removed the peer-review-needed Signifies that the peer review team needs to review this PR label Dec 7, 2021
@lpettyjo lpettyjo added the peer-review-done Signifies that the peer review team has reviewed this PR label Dec 7, 2021
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 7, 2021
@zhouying7780
Copy link

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 8, 2021
@kmccarron-rh
Copy link
Contributor

lgtm!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.9 lgtm Indicates that a PR is ready to be merged. peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants