fixing grammar future tense to present tense, removing pronouns #8644
Conversation
openshift-ci-robot
added
the
size/L
admin_guide/diagnostics_tool.adoc
Outdated
| the diagnostics were added to the `openshift` binary so that wherever there is | ||
| an {product-title} server or client, the diagnostics can run in the exact same | ||
| environment. | ||
| {product-title} may be deployed in numerous scenarios including: |
There was a problem hiding this comment.
You can deploy {product-title} by using several methods:
There was a problem hiding this comment.
Do we want to start sentences with You?
There was a problem hiding this comment.
Yes. Our style guide says that docs are supposed to favor active, user-focused sentences. It's always better to say "you," meaning the user, "do <the thing>" instead of "<The thing> can be done."
admin_guide/diagnostics_tool.adoc
Outdated
| environment. | ||
| {product-title} may be deployed in numerous scenarios including: | ||
|
|
||
| * built from source |
There was a problem hiding this comment.
Capitalize the first letter in each line of the list.
admin_guide/diagnostics_tool.adoc
Outdated
| * as a container image | ||
| * via enterprise RPMs | ||
|
|
||
| Each method implies a different configuration and environment. The diagnostics |
There was a problem hiding this comment.
s/implies/is suited for
s/The diagnostics were included within openshift binary/The diagnostic tool is included in the {project-title} binary file
s/assumptions and provide the ability to run the diagnostics tool within/assumptions. You can run the diagnostics tool from
admin_guide/diagnostics_tool.adoc
Outdated
|
|
||
| To use the diagnostics tool, preferably on a master host and as cluster | ||
| administrator, run: | ||
| administrator, run a `sudo` user: |
admin_guide/diagnostics_tool.adoc
Outdated
| ---- | ||
|
|
||
| This runs all available diagnostics, skipping any that do not apply. | ||
| The above command runs all available diagnostis skipping any that do not apply |
There was a problem hiding this comment.
s/above/previous
s/diagnostis skipping/diagnostics and skips
admin_guide/diagnostics_tool.adoc
Outdated
| Keep this in mind for these volume mount specifications because it could have | ||
| unexpected consequences. For example, if you mount (and therefore relabel) your | ||
| *_$HOME/.ssh_* directory, *sshd* will become unable to access your public keys | ||
| It is important to note these volume mount specifications because it could have |
admin_guide/diagnostics_tool.adoc
Outdated
| unexpected consequences. For example, if you mount (and therefore relabel) your | ||
| *_$HOME/.ssh_* directory, *sshd* will become unable to access your public keys | ||
| It is important to note these volume mount specifications because it could have | ||
| unexpected consequences. For example, if one mounts (and therefore relabels) the |
There was a problem hiding this comment.
s/one mounts (and therefore relabels)/you mount, and therefore relabel,
admin_guide/diagnostics_tool.adoc
Outdated
| *_$HOME/.ssh_* directory, *sshd* will become unable to access your public keys | ||
| It is important to note these volume mount specifications because it could have | ||
| unexpected consequences. For example, if one mounts (and therefore relabels) the | ||
| *_$HOME/.ssh_* directory, *sshd* becomes unable to access the public keys |
admin_guide/diagnostics_tool.adoc
Outdated
| a *_known_hosts_* file and have SSH validate host keys, which is disabled by the | ||
| default configuration and can be re-enabled with an environment variable by | ||
| adding `-e ANSIBLE_HOST_KEY_CHECKING=True` to the `docker` command line. | ||
| It is plausible to want to mount an entire *_.ssh_* directory for various |
There was a problem hiding this comment.
If you mount an entire .ssh directory:
- You can use an SSH configuration to match keys with hosts or modify other connection parameters.
- You can provide a known_hosts file and use SSH to validate host keys. To use this feature, which
+is disabled by default, use thedockercommand line to add the-e ANSIBLE_HOST_KEY_CHECKING=Trueenvironment variable to the container.
admin_guide/diagnostics_tool.adoc
Outdated
| Standard location of the configuration files placed by an Ansible-deployed | ||
| cluster ensures that running `sudo oc adm diagnostics` works without any flags. | ||
| In the event, the standard location of the configuration files is not used, | ||
| options flags as those listed in the example below may be used. |
There was a problem hiding this comment.
s/example below/following example
openshift-bot
added
the
needs-rebase
|
@rlopez133 - will you be updating this PR based on @kalexand-rh's changes? Please make sure to rebase before you do. |
openshift-ci-robot
added
size/M
openshift-bot
removed
the
needs-rebase
openshift-ci-robot
added
size/L
|
Sorry, I should've noted that this PR came before my PR of #8741 and this was left open so we could discuss it (over email), and I created #8741 out of that discussion. I'd suggest this PR be closed. That said, I can see that there was a rebase (that probably took time) so it's up to @rlopez133 and maybe @vikram-redhat to determine what to do. |
|
@bfallonf - is this undoing any of the work in 8741? |
|
@vikram-redhat Merging this would undo some of 8741, yes. |
|
I'm just trying to help out :) I did the rebase (yes it did take sometime to do all that). And then I incorporated all the changes that @kalexand-rh wanted which were quite a few. I'm open to suggestions as long as it doesn't mean re-doing all those changes because it was a lot of changes... |
|
Hey @rlopez133 - all that help is really appreciated :). We just need to figure out the best way to merge this PR so that we don't override 8741. @bfallonf let's look at this in the office today. |
|
@vikram-redhat @bfallonf - Also when I did the rebase, I did do a "this sounds the best" when having to make some decisions... I won't be offended if you want to change it back or to something else. Probably my biggest thing is starting sentences with pronouns like "You" but to each his own :) |
|
I'll paste the above msg from @kalexand-rh here, cos it's worded well:
Big thanks again @rlopez133 ! I'll do a quick followup PR. |
|
Followup PR at #10983 I'll close this PR. If anyone has thoughts, please let me know. |
…-10983-to-enterprise-3.10 [enterprise-3.10] Edits after #8644
…-10983-to-enterprise-3.9 [enterprise-3.9] Edits after #8644
Fixing some grammatical mistakes, removing use of future tense wording and keeping it present tense i.e. will. Removed use of pronouns i.e. you, your. Included sudo to the diagnostic commands as a certain amount of privilege is required to run the commands.