Skip to content

Use Optional: instead#30835

Merged
jboxman merged 1 commit intoopenshift:masterfrom
jboxman-rh:fix-optional-style
Mar 24, 2021
Merged

Use Optional: instead#30835
jboxman merged 1 commit intoopenshift:masterfrom
jboxman-rh:fix-optional-style

Conversation

@jboxman
Copy link
Contributor

@jboxman jboxman commented Mar 24, 2021

Always use Optional:.

@jboxman jboxman added this to the Next Release milestone Mar 24, 2021
@jboxman jboxman self-assigned this Mar 24, 2021
@jboxman jboxman added the peer-review-needed Signifies that the peer review team needs to review this PR label Mar 24, 2021
@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 24, 2021
@netlify
Copy link

netlify bot commented Mar 24, 2021

Deploy preview for osdocs ready!

Built with commit f37791b

https://deploy-preview-30835--osdocs.netlify.app

Copy link
Contributor

@bergerhoffer bergerhoffer left a comment

Choose a reason for hiding this comment

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

Actual changes look good, but have two additional pieces of feedback:

  1. I don't see in our guidelines that "Optional:" is preferred, though I remember talking about this, so I'm surprised that it's not. If we're asking people to use that over other ways, we should add it as a guideline. Would you mind doing that in a separate PR?
  2. I would backport this as far as possible, just to avoid future merge conflicts for others.

@bergerhoffer bergerhoffer 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 Mar 24, 2021
@jboxman
Copy link
Contributor Author

jboxman commented Mar 24, 2021

@bergerhoffer, from what I gather, this came from ISG and in the new edition appears on page 185.

@bergerhoffer
Copy link
Contributor

@bergerhoffer, from what I gather, this came from ISG and in the new edition appears on page 185.

Awesome, I hadn't caught that in the update. That's good enough for me then, thanks!

@jboxman jboxman merged commit 6715d11 into openshift:master Mar 24, 2021
@jboxman jboxman deleted the fix-optional-style branch March 24, 2021 18:15
@jboxman
Copy link
Contributor Author

jboxman commented Mar 24, 2021

/cherry-pick enterprise-4.8

@openshift-cherrypick-robot

@jboxman: new pull request created: #30864

Details

In response to this:

/cherry-pick enterprise-4.8

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.

@jboxman
Copy link
Contributor Author

jboxman commented Mar 24, 2021

/cherry-pick enterprise-4.7

@openshift-cherrypick-robot

@jboxman: new pull request created: #30865

Details

In response to this:

/cherry-pick enterprise-4.7

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.

@jboxman
Copy link
Contributor Author

jboxman commented Mar 24, 2021

/cherry-pick enterprise-4.6

@openshift-cherrypick-robot

@jboxman: #30835 failed to apply on top of branch "enterprise-4.6":

Applying: Use Optional: instead
Using index info to reconstruct a base tree...
M	modules/cnf-provisioning-real-time-and-low-latency-workloads.adoc
M	modules/ipi-install-node-requirements.adoc
M	modules/virt-confirming-policy-updates-on-nodes.adoc
M	modules/virt-creating-vm-wizard-web.adoc
M	modules/virt-creating-vm-yaml-web.adoc
M	updating/updating-cluster-rhel-compute.adoc
Falling back to patching base and 3-way merge...
Auto-merging updating/updating-cluster-rhel-compute.adoc
Auto-merging modules/virt-creating-vm-yaml-web.adoc
Auto-merging modules/virt-creating-vm-wizard-web.adoc
CONFLICT (content): Merge conflict in modules/virt-creating-vm-wizard-web.adoc
Auto-merging modules/virt-confirming-policy-updates-on-nodes.adoc
Auto-merging modules/ipi-install-node-requirements.adoc
Auto-merging modules/cnf-provisioning-real-time-and-low-latency-workloads.adoc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Use Optional: instead
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherry-pick enterprise-4.6

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.

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

Labels

branch/enterprise-4.7 branch/enterprise-4.8 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.

4 participants