Skip to content

Conversation

@aburdenthehand
Copy link
Contributor

@aburdenthehand aburdenthehand commented Jul 28, 2020

Known issue plus workaround for common-templates bug

Docs bug: https://bugzilla.redhat.com/show_bug.cgi?id=1861322
Eng bug: https://bugzilla.redhat.com/show_bug.cgi?id=1861297

@rmohr can you please review

Edit: adding screenshot of build
Screenshot from 2020-07-28 16-11-39

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 28, 2020
Copy link

@rmohr rmohr left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 28, 2020
Copy link
Member

@ousleyp ousleyp left a comment

Choose a reason for hiding this comment

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

LGTM, I just have some nits.

Copy link
Member

Choose a reason for hiding this comment

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

s/creating/you create

s/using/by using

Question: should 0 be in backticks if you're referring to the value in the yaml?

Suggestion: We generally don't use parenthesis. Instead of (force stop), could you very briefly explain why 0 = force stop?

Copy link
Member

Choose a reason for hiding this comment

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

nit/suggestion: I'd swap this for Edit the common template by running the following command:

Copy link
Member

Choose a reason for hiding this comment

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

s/will be granted/is granted
s/gracefully shutdown/gracefully shut down

@aburdenthehand aburdenthehand force-pushed the cnv-bz1861322-graceperiod branch from 4728b14 to 3c1454c Compare July 28, 2020 16:22
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 28, 2020
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@aburdenthehand
Copy link
Contributor Author

QE ackd in a pre-release jira (cnv-4934)

@aburdenthehand aburdenthehand merged commit b136c71 into openshift:enterprise-4.5 Jul 28, 2020
@aburdenthehand aburdenthehand deleted the cnv-bz1861322-graceperiod branch July 12, 2021 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

4 participants