Skip to content
This repository has been archived by the owner on Jul 12, 2023. It is now read-only.

Doc cleanup largely based on spelling/grammar check #724

Merged
merged 2 commits into from
Oct 1, 2020

Conversation

greggles
Copy link
Contributor

@greggles greggles commented Sep 30, 2020

As I was reading through these I noticed some spelling issues at the end of the realm_guide. In training with some users, they mentioned it was confusing setting up 2fa due to the need to prefix the phone number with +1 and suggested adding a note about that to the docs. I also ran the 2 files through grammarly and accepted most of the suggestions. Some items felt like personal preference not worth changing.

Proposed Changes

  • Fix spelling issues in realm_guide
  • Explain multiple 2fa phones are useful
  • General small fixes

Release Note

NONE

@google-cla google-cla bot added the cla: yes Auto: added by CLA bot when all committers have signed a CLA. label Sep 30, 2020
@googlebot googlebot added the cla: yes Auto: added by CLA bot when all committers have signed a CLA. label Sep 30, 2020
@google-oss-robot
Copy link

Hi @greggles. Thanks for your PR.

I'm waiting for a google member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@@ -110,15 +109,15 @@ Click the "Create a new signing key version" button. This will _create_ but not

![api keys](images/admin/keys01.png "API key created")

If successful, you will get a message indiciating the new key version that was created.
If successful, you will get a message indicating the new key version that was created.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here to the end of the file are some spelling fixes.

This will provide a link to set up your account password.

![new user](images/users/step02.png "Create a password")

### Second factor authentication

On your next login, you will be given the option to enroll a second factor for authentication (SMS sent to your personal mobile phone). It is highly recommended to enroll the second factor.
On your next login, you will be given the option to enroll a second factor for authentication (SMS sent to your mobile phone). It is highly recommended to enroll in a second factor. The phone number requires the international prefix (e.g. +1 for USA).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if it's appropriate to put in USA specific advice. The overall idea was suggested by a few admins who noted they were confused by this input.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On double-checking the form it looks like maybe the input widget was improved in a way that makes this change unneccessary. Happy to remove this if you agree.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove. We added the nicer widget just this week #702 because this is a known point of confusion

Copy link
Contributor

Choose a reason for hiding this comment

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

We could also note that more-than-one second factor can be useful in case the user loses access to their primary device.

Copy link
Contributor

@whaught whaught left a comment

Choose a reason for hiding this comment

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

/ok-to-test

And welcome! Thanks for helping out.

@icco
Copy link
Contributor

icco commented Oct 1, 2020

/approved

I'll let @whaught put the lgtm on, looks great though, thank you!

@greggles
Copy link
Contributor Author

greggles commented Oct 1, 2020

Cool, now addressed those suggested changes and udpated the summary. Happy to hear any more feedback if there is any.

Copy link
Member

@sethvargo sethvargo left a comment

Choose a reason for hiding this comment

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

/lgtm

@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: greggles, sethvargo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-robot google-oss-robot merged commit e2731f2 into google:main Oct 1, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes Auto: added by CLA bot when all committers have signed a CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants