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

Clarify access level needed for secrets in web interface #2330

Merged
merged 4 commits into from
Mar 4, 2021

Conversation

muru
Copy link
Contributor

@muru muru commented Dec 25, 2020

Why:

Closes #1087

What's being changed:

I changed the initial text of "Creating encrypted secrets for a repository" and also added a note at the end of that section:

  • Including a mention of "web interface" lead to (IMO) too much duplication in text, so I rephrased it to be more like the permissions-statement-secrets-api reusable.

  • I considered changing the permissions-statement-secrets-repository reusable to include a reference to the API, but then I noticed that the other place using it ("Enabling debug logging") already mentioned the API, so instead I added a note.

  • Screenshot of "Creating encrypted secrets for a repository", with the unchanged parts snipped:

    screenshot showing the initial paragraph and final note with the unchanged middle omitted

  • Screenshot of "Enabling debug logging":

    screenshot showing entire list of permission requirements with change highlighted

Check off the following:

  • I have reviewed my changes in staging. (look for the deploy-to-heroku link in your pull request, then click View deployment)
  • For content changes, I have reviewed the localization checklist
  • For content changes, I have reviewed the Content style guide for GitHub Docs.
    • The style guide says "do not include [...] permissions," however I feel that this note might be acceptable as "useful information or reminders for the user" since it describes a relaxation in permissions compared to the preceding text.

Closes github#1087

I considered changing the `permissions-statement-secrets-repository`
reusable to include a reference to the API, but then I noticed that the
other place using it (["Enabling debug logging"][1]) already mentioned
the API, so instead I added a note. Including a mention of "web
interface" lead to (IMO) too much duplication in text, so I rephrased it
to be more like the `permissions-statement-secrets-api` reusable.

[1]: https://docs.github.com/en/free-pro-team@latest/actions/managing-workflow-runs/enabling-debug-logging
@welcome
Copy link

welcome bot commented Dec 25, 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.

loy8888
loy8888 previously approved these changes Dec 28, 2020
@janiceilene
Copy link
Contributor

Thanks so much for opening a PR @muru! I'll get this triaged for the team to take a look when they're back from the holidays 💖

@janiceilene janiceilene added content This issue or pull request belongs to the Docs Content team ecosystem This issue or pull request should be reviewed by the Docs Ecosystem team labels Dec 28, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Jan 4, 2021

This PR is stale because it has been open 7 days with no activity and will be automatically closed in 3 days. To keep this PR open, update the PR by adding a comment or pushing a commit.

@github-actions github-actions bot added the stale There is no recent activity on this issue or pull request label Jan 4, 2021
@janiceilene janiceilene removed the stale There is no recent activity on this issue or pull request label Jan 5, 2021
@janiceilene
Copy link
Contributor

Thanks for your patience! Our small team is working our way through reviewing all of the amazing contributions ✨

@github-actions
Copy link
Contributor

This PR is stale because it has been open 7 days with no activity and will be automatically closed in 3 days. To keep this PR open, update the PR by adding a comment or pushing a commit.

@github-actions github-actions bot added the stale There is no recent activity on this issue or pull request label Jan 16, 2021
@shati-patel shati-patel removed the stale There is no recent activity on this issue or pull request label Jan 19, 2021
@github-actions
Copy link
Contributor

This PR is stale because it has been open 7 days with no activity and will be automatically closed in 3 days. To keep this PR open, update the PR by adding a comment or pushing a commit.

@github-actions github-actions bot added the stale There is no recent activity on this issue or pull request label Jan 26, 2021
@janiceilene janiceilene removed the stale There is no recent activity on this issue or pull request label Jan 26, 2021
@janiceilene janiceilene added the waiting for review Issue/PR is waiting for a writer's review label Feb 5, 2021
@github-actions github-actions bot closed this Feb 13, 2021
@muru
Copy link
Contributor Author

muru commented Feb 15, 2021

Is the closure intentional? Should I amend the PR?

@janiceilene
Copy link
Contributor

I'm so sorry about that @muru! No amendments needed. It looks like our new action inadvertently closed this 🙃 Reopening and triaging this for review 💛

@janiceilene janiceilene reopened this Feb 16, 2021
@github-actions github-actions bot removed the waiting for review Issue/PR is waiting for a writer's review label Feb 17, 2021
@chiedo chiedo added the waiting for review Issue/PR is waiting for a writer's review label Feb 17, 2021
Copy link
Contributor

@hubwriter hubwriter left a comment

Choose a reason for hiding this comment

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

@muru - Many thanks for raising this PR.

I like the note that you added, with the link to the secrets API page, that's useful, thanks.

I think we should stick with the current reusable text. IMO it makes a clearer distinction between what you need to create a secret for a user account repo and what you need to create a secret for an organization repo.

…pository.md


I'm going to go ahead and change this back.
@hubwriter hubwriter dismissed stale reviews from ghost and loy8888 via e7445c6 March 3, 2021 17:15
hubwriter
hubwriter previously approved these changes Mar 3, 2021
Copy link
Contributor

@hubwriter hubwriter left a comment

Choose a reason for hiding this comment

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

@muru - As this PR has been waiting for our attention for so long I'm going to go ahead and merge the modified version (without the change to the reusable text).

Many thanks again for taking the time to help improve our docs. It's much appreciated. 👍

Of course if you disagree with my comments please get back in touch.
All the best.

@muru
Copy link
Contributor Author

muru commented Mar 3, 2021

@hubwriter that's fine by me! Thanks for reviewing!

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 ecosystem This issue or pull request should be reviewed by the Docs Ecosystem team waiting for review Issue/PR is waiting for a writer's review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarify permission level needed to access secrets
6 participants