Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update getting-started.md #515

Open
wants to merge 2 commits into
base: master
from
Open

Update getting-started.md #515

wants to merge 2 commits into from

Conversation

@sennap
Copy link

@sennap sennap commented Oct 1, 2019

As someone who went through this process for the first time, I wanted to make note of some things in the guide that I was confused by, and propose some wording that might help save others from questions:

  • Added note about creating a new backup.config file + example command to do that =
  • Added ./ for running those ghe commands

cc @skhalife @glennwester @nielspineda

@sennap sennap requested a review from lildude Oct 1, 2019
@@ -31,13 +34,13 @@
* In a clustering environment, the `GHE_EXTRA_SSH_OPTS` key must be configured
with the `-i <abs path to private key>` SSH option.

3. Add the backup host's SSH key to the GitHub appliance as an *Authorized SSH
3. Add the backup host's (in this case your computer) SSH key to the GitHub appliance as an *Authorized SSH

This comment has been minimized.

@lildude

lildude Oct 3, 2019
Member

I'm not sure I understand this addition. To me, this addition suggests the SSH key the admin user uses to connect to the backup host is the one that needs to be added to the appliance, which isn't necessarily correct.

We encourage customers to use a dedicated host for their backups and if they're using good practices, the backup user will have their own SSH key which won't be the same as the one the admin uses.

Could you explain this addition a bit more so I can understand the background behind its addition.

This comment has been minimized.

@sennap

sennap Dec 22, 2019
Author

hey @lildude 👋 ! sorry I completely forgot about this PR 🙈

I'm having a hard time remembering what my logic was there and I agree with what you're saying. It doesn't really make sense so I'm removing this line!

@djdefi
djdefi approved these changes Feb 11, 2020
@djdefi djdefi mentioned this pull request Feb 11, 2020
Copy link

@caosang1988 caosang1988 left a comment

@oakeyc
Copy link
Contributor

@oakeyc oakeyc commented Sep 2, 2020

@sennap would you still like this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.