Skip to content

Conversation

@chinmayi-chandrasekar
Copy link
Contributor

@chinmayi-chandrasekar chinmayi-chandrasekar commented Sep 14, 2021

@openshift-ci
Copy link

openshift-ci bot commented Sep 14, 2021

@chinmayi-chandrasekar: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 14, 2021
@netlify
Copy link

netlify bot commented Sep 14, 2021

✔️ Deploy Preview for osdocs ready!

🔨 Explore the source changes: 895dd1a

🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/614ac050c6d76c000732e51c

😎 Browse the preview: https://deploy-preview-36324--osdocs.netlify.app/openshift-enterprise/latest/release_notes/ocp-4-6-release-notes

@chinmayi-chandrasekar chinmayi-chandrasekar changed the base branch from main to enterprise-4.6 September 14, 2021 10:45
@openshift-ci openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 14, 2021
@sagidlow sagidlow added this to the Next Release milestone Sep 14, 2021
@sagidlow sagidlow removed branch/enterprise-4.7 branch/enterprise-4.8 branch/enterprise-4.9 needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 14, 2021
Copy link
Contributor

@sagidlow sagidlow left a comment

Choose a reason for hiding this comment

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

I left two comments, otherwise LGTM.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
==== TLS verification falling back to the Common Name field
==== TLS verification falling back to the *Common Name* field

Per the IBM Style Guide, we should be bolding field names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The behavior of falling back to the Common Name field on X.509 certificates as a host name when no Subject Alternative Names are present is removed. Certificates must properly set the Subject Alternative Names field.
The behavior of falling back to the *Common Name* field on X.509 certificates as a host name when no Subject Alternative Names are present is removed. Certificates must properly set the *Subject Alternative Names* field.

Per the IBM Style Guide, we should be bolding field names.

Subject Alternative Names are capitalized like acronyms. If it is not a proper noun then it needs to be in lower case. If it is a field, then it needs to be bolded and identified as a field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed.

@sagidlow sagidlow added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Sep 14, 2021
@chinmayi-chandrasekar chinmayi-chandrasekar force-pushed the BZ1997337_SAN_certificate_feature_RN_4.6 branch from 25b0ec9 to a73e5d5 Compare September 14, 2021 12:48

Choose a reason for hiding this comment

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

suggest to add keyword 'removed', like 'TLS verification falling back to the Common Name field removed'

@chinmayi-chandrasekar chinmayi-chandrasekar force-pushed the BZ1997337_SAN_certificate_feature_RN_4.6 branch from a73e5d5 to e2e8b2a Compare September 15, 2021 06:23
@sttts
Copy link

sttts commented Sep 20, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 20, 2021
@yaoli-redhat
Copy link

/lgtm

@chinmayi-chandrasekar chinmayi-chandrasekar force-pushed the BZ1997337_SAN_certificate_feature_RN_4.6 branch from e2e8b2a to 895dd1a Compare September 22, 2021 05:34
@openshift-ci
Copy link

openshift-ci bot commented Sep 22, 2021

New changes are detected. LGTM label has been removed.

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 22, 2021
@vikram-redhat
Copy link
Contributor

Since the support status of a feature is changing after GA, we need to follow change management for this.

@bergerhoffer bergerhoffer merged commit 62e32c6 into openshift:enterprise-4.6 Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.6 peer-review-done Signifies that the peer review team has reviewed this PR size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants