Skip to content

Conversation

@rh-max
Copy link
Contributor

@rh-max rh-max commented Oct 1, 2019

This PR implements Junqi's (QE) feedback for the 4.2 monitoring docs from #16586, #16752, #16474, and #16575. Only a few items were not implemented fully or at all (I commented on those items with explanations).
@juzhao Thank you for the review!

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 1, 2019
@openshift-docs-preview-bot

The preview will be available shortly at:

@rh-max rh-max force-pushed the monitoring-4.2-qe-feedback branch from ad55731 to b79a384 Compare October 9, 2019 15:12
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 9, 2019
Copy link
Contributor

@jboxman jboxman left a comment

Choose a reason for hiding this comment

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

(My first use of the review button.)

One comment, but 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.

Per our style guide, we're replacing spaces with underscores: <foo_variable>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented everywhere in monitoring docs.

@jboxman
Copy link
Contributor

jboxman commented Oct 9, 2019

Oh, and there is one build failure:

>>> Working on monitoring book in openshift-enterprise <<<
Transforming the AsciiDoc content to DocBook XML...
Specification mandates value for attribute for, line 245, column 26 (line 245)
>>> Finished with monitoring book in openshift-enterprise <<<

@jboxman jboxman self-assigned this Oct 9, 2019
@jboxman jboxman added peer-review-done Signifies that the peer review team has reviewed this PR branch/enterprise-4.2 labels Oct 9, 2019
@jboxman jboxman added this to the Future Release milestone Oct 9, 2019
@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 9, 2019
@rh-max rh-max force-pushed the monitoring-4.2-qe-feedback branch from cf3ddd5 to f4a6878 Compare October 9, 2019 22:59
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 9, 2019
@jboxman
Copy link
Contributor

jboxman commented Oct 10, 2019

@rh-max, can you squash into a single commit and force-push? Then I'll merge. Thanks!

@rh-max rh-max force-pushed the monitoring-4.2-qe-feedback branch from 027bb68 to 7b2d596 Compare October 10, 2019 13:25
@rh-max
Copy link
Contributor Author

rh-max commented Oct 10, 2019

@jboxman Done. Thank you!

Make codeblock markup comply with conventions

More codeblock markup fixes

Try to fix markup

Update a link
@rh-max rh-max force-pushed the monitoring-4.2-qe-feedback branch from 85ab5e6 to 27e5b5f Compare October 14, 2019 10:33
@rh-max
Copy link
Contributor Author

rh-max commented Oct 14, 2019

@jboxman I've made one other change:
https://github.com/openshift/openshift-docs/pull/16978/files#diff-cf9d582967542fd125f5df9012e5c2dbL42
It's all squashed now, so you can merge & cherry-pick. Thanks!

@jboxman jboxman merged commit b594348 into openshift:master Oct 14, 2019
@jboxman
Copy link
Contributor

jboxman commented Oct 14, 2019

/cherrypick enterprise-4.2

@openshift-cherrypick-robot

@jboxman: new pull request created: #17275

Details

In response to this:

/cherrypick enterprise-4.2

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.

@rh-max
Copy link
Contributor Author

rh-max commented Oct 14, 2019

Thank you @jboxman !

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

Labels

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

6 participants