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

Specify ssh shell is required to enable dependency graph #538

Merged
merged 10 commits into from
Oct 22, 2020

Conversation

djdefi
Copy link
Member

@djdefi djdefi commented Oct 14, 2020

Why:

Adds clarification that the ghe-dep-graph-enable command needs to be run within the administrative SSH shell, and links to the relevant document for SSH access.

What's being changed:

Calls out the requirement to run the ghe-dep-graph-enable command via SSH and links to the relevant section for setting up SSH shell access

Check off the following:

Specify that the command should be run within the administrative SSH shell, and link to the relevant document for access.
@welcome
Copy link

welcome bot commented Oct 14, 2020

Thanks for opening this pull request! A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines.

@@ -41,7 +41,7 @@ Before enabling {% if currentVersion ver_gt "enterprise-server@2.21" %}{% data v
{% endif %}

{% data reusables.enterprise_site_admin_settings.sign-in %}
1. In the administrative shell, enable the {% if currentVersion ver_gt "enterprise-server@2.21" %}{% data variables.product.prodname_dependabot_short %}{% else %}security{% endif %} alerts for vulnerable dependencies on {% data variables.product.product_location_enterprise %}:
1. In the [administrative SSH shell](/enterprise/{{ currentVersion }}/admin/configuration/accessing-the-administrative-shell-ssh#enabling-access-to-the-administrative-shell-via-ssh), enable the {% if currentVersion ver_gt "enterprise-server@2.21" %}{% data variables.product.prodname_dependabot_short %}{% else %}security{% endif %} alerts for vulnerable dependencies on {% data variables.product.product_location_enterprise %}:
Copy link
Member Author

Choose a reason for hiding this comment

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

I realize this is an in-line link which is not preferred, but it felt a bit long as a "For more information on SSH access:" type call out.

Copy link
Contributor

@janiceilene janiceilene Oct 15, 2020

Choose a reason for hiding this comment

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

👋 @djdefi Thanks so much for opening a PR! I agree that it'd be too long to use add a For more info into the procedural step, but we avoid inline links to help with localization, accessibility, and ensure users always understand what they're going to click on. What if, instead of using an inline link, you added a note after the shell command on line 47. Something like:

{{#note}}

**Note**: For more information about enabling access to the administrative shell via SSH, see "[Enabling alerts for vulnerable dependencies on GitHub Enterprise Server](/enterprise/{{ currentVersion }}/admin/configuration/accessing-the-administrative-shell-ssh#enabling-access-to-the-administrative-shell-via-ssh)."

{{/note}}

@@ -41,7 +41,7 @@ Before enabling {% if currentVersion ver_gt "enterprise-server@2.21" %}{% data v
{% endif %}

{% data reusables.enterprise_site_admin_settings.sign-in %}
1. In the administrative shell, enable the {% if currentVersion ver_gt "enterprise-server@2.21" %}{% data variables.product.prodname_dependabot_short %}{% else %}security{% endif %} alerts for vulnerable dependencies on {% data variables.product.product_location_enterprise %}:
1. In the [administrative SSH shell](/enterprise/{{ currentVersion }}/admin/configuration/accessing-the-administrative-shell-ssh#enabling-access-to-the-administrative-shell-via-ssh), enable the {% if currentVersion ver_gt "enterprise-server@2.21" %}{% data variables.product.prodname_dependabot_short %}{% else %}security{% endif %} alerts for vulnerable dependencies on {% data variables.product.product_location_enterprise %}:
Copy link
Contributor

@janiceilene janiceilene Oct 15, 2020

Choose a reason for hiding this comment

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

👋 @djdefi Thanks so much for opening a PR! I agree that it'd be too long to use add a For more info into the procedural step, but we avoid inline links to help with localization, accessibility, and ensure users always understand what they're going to click on. What if, instead of using an inline link, you added a note after the shell command on line 47. Something like:

{{#note}}

**Note**: For more information about enabling access to the administrative shell via SSH, see "[Enabling alerts for vulnerable dependencies on GitHub Enterprise Server](/enterprise/{{ currentVersion }}/admin/configuration/accessing-the-administrative-shell-ssh#enabling-access-to-the-administrative-shell-via-ssh)."

{{/note}}

@janiceilene janiceilene added content This issue or pull request belongs to the Docs Content team core labels Oct 15, 2020
djdefi and others added 3 commits October 21, 2020 09:50
…endencies-on-github-enterprise-server.md

Co-authored-by: hubwriter <54933897+hubwriter@users.noreply.github.com>
I should have spotted this in my previous review, so I'm going to go ahead and apply this change.
@hubwriter
Copy link
Contributor

@djdefi - Many thanks for raising this PR and helping to improve GitHub's documentation. 👍
I'm merging this now.

@hubwriter hubwriter merged commit 29a9e63 into main Oct 22, 2020
@hubwriter hubwriter deleted the djdefi-dep-graph-shell branch October 22, 2020 08:55
@github-actions
Copy link
Contributor

Thanks very much for contributing! Your pull request has been merged 🎉 You should see your changes appear on the site in approximately 24 hours.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content This issue or pull request belongs to the Docs Content team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants