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

Style guide for access, permission, role names and its usage unclear #31686

Closed
1 task done
janbrasna opened this issue Feb 16, 2024 · 8 comments · Fixed by #31898
Closed
1 task done

Style guide for access, permission, role names and its usage unclear #31686

janbrasna opened this issue Feb 16, 2024 · 8 comments · Fixed by #31898
Labels
content This issue or pull request belongs to the Docs Content team contributing docs Content related to our contributing docs help wanted Anyone is welcome to open a pull request to fix this issue

Comments

@janbrasna
Copy link
Contributor

Code of Conduct

What article on docs.github.com is affected?

https://docs.github.com/en/contributing/style-guide-and-content-model/style-guide#permissions

What part(s) of the article would you like to see updated?

The chapter defines role usage in caps & quoted:

Resources (e.g., "Write" for a repository, "Admin" for a security advisory)

yet the examples below don't use any of that:

Avoid: People with the write role can...

Trying to follow the Style guide, I'd be tempted to use:

Avoid: People with "Write" role can...

Is there a clear style to follow? I'd like to adhere in PRs as much as possible.

Additional information

@janbrasna janbrasna added the content This issue or pull request belongs to the Docs Content team label Feb 16, 2024
@github-actions github-actions bot added the triage Do not begin working on this issue until triaged by the team label Feb 16, 2024
@nguyenalex836 nguyenalex836 added contributing docs Content related to our contributing docs and removed triage Do not begin working on this issue until triaged by the team labels Feb 19, 2024
@nguyenalex836
Copy link
Contributor

@janbrasna Thank you for opening this issue, as well as providing additional context on this issue alongside the other PR you opened! ✨ I'll get this triaged for review 💛

@nguyenalex836 nguyenalex836 added the waiting for review Issue/PR is waiting for a writer's review label Feb 19, 2024
@felicitymay felicitymay added the needs SME This proposal needs review from a subject matter expert label Feb 22, 2024
Copy link
Contributor

Thanks for opening an issue! We've triaged this issue for technical review by a subject matter expert 👀

@ethanpalm
Copy link
Contributor

Hi @janbrasna, thanks for asking this question and opening an issue to clear up this confusing part of the style guide.

The definition is misleading here and the terms should not be capitalized or use quotation marks. The examples like

People with write access can

are the proper style.

I reviewed the rest of the style guide to make sure that there aren't other examples like this that use quotation marks to introduce terms that don't need them and it looks like this is the only one. Removing the quotation marks and making the terms lower case in the permissions section should make this more clear 😄 .

You or anyone else is welcome to open a pull request to make these changes!

@ethanpalm ethanpalm added the help wanted Anyone is welcome to open a pull request to fix this issue label Feb 22, 2024
@ethanpalm ethanpalm removed waiting for review Issue/PR is waiting for a writer's review needs SME This proposal needs review from a subject matter expert labels Feb 22, 2024
@janbrasna
Copy link
Contributor Author

janbrasna commented Feb 23, 2024

@ethanpalm Thanks for the info. I'll look into it, just need a bit guidance here if possible:

  1. I've seen occurrences of "people with the write access" which sounds weird — this should be probably mentioned in the examples to avoid?

  2. I originally came from Quote role names in rulesets #31532 where the sentence in question felt weird (to me, probably as a non–native speaker):

    "The following are eligible for bypass access: - The maintain or write role, or custom repository roles based on the write role"

    so just checking this wording is correct. (No problem with "with whatever access", just the analogous "the foo or bar role" sounded puzzling. Maybe the definite article here is the culprit? Should the sentences about roles be reworded like "People with maintain or write roles,…" to get rid of the article? — Sorry I suck in English so might need a little more hand-holding fixing this than a proper content person would need;)

Also relevant from Style guide and content model: https://docs.github.com/en/contributing/style-guide-and-content-model/contents-of-a-github-docs-article#how-to-write-a-permissions-statement

@gsosm0760

This comment was marked as spam.

@ethanpalm
Copy link
Contributor

@janbrasna Great questions. With all the different options for access, permissions, and roles, there are a lot of challenges to accurately and clearly write about these terms.

  1. I've seen occurrences of "people with the write access" which sounds weird — this should be probably mentioned in the examples to avoid?

I agree using "the write access" sounds incorrect. This would be a good additional example in the style guide of what to avoid after the first similar example.

  1. I originally came from Quote role names in rulesets #31532 where the sentence in question felt weird (to me, probably as a non–native speaker):

    "The following are eligible for bypass access: - The maintain or write role, or custom repository roles based on the write role"

    so just checking this wording is correct. (No problem with "with whatever access", just the analogous "the foo or bar role" sounded puzzling. Maybe the definite article here is the culprit? Should the sentences about roles be reworded like "People with maintain or write roles,…" to get rid of the article? — Sorry I suck in English so might need a little more hand-holding fixing this than a proper content person would need;)

It looks like this specific article has several list items that could be rewritten to be more clear and closer to our style guide. And I think you're right that the definite article is what's causing a lot of confusion in that particular line.

The first item:

  • Repository admins or organization owners

Since these roles are at different levels (repository-level and organization-level), they should be separate list items. It is confusing to have them together since we wouldn't do that elsewhere in the Docs 😅 .

The second item:

  • The maintain or write role, or custom repository roles based on the write role

This should be rewritten like you suggested to get rid of the definite article. We might also be able to rewrite it like "People with write access to the repository" since that would include all the repository level roles (admin, maintain, write, and custom roles). I'd want to first confirm that this is what actually allows bypass access since I'm not the most knowledgeable on this feature, but based on who can get bypass access, it looks like it's controlled through write access.

And your English is fantastic 😄 I appreciate all the questions you are asking to help make things more clear and improve the Docs overall.

@CBID2 CBID2 mentioned this issue Feb 29, 2024
2 tasks
@CBID2
Copy link
Contributor

CBID2 commented Feb 29, 2024

Hi @ethanpalm! :) I created a pull request for this issue. It's #31898. So far I added the avoid example like you suggested, and I'm not sure what else to put. Can you help me?

@ethanpalm
Copy link
Contributor

Hi @ethanpalm! :) I created a pull request for this issue. It's #31898. So far I added the avoid example like you suggested, and I'm not sure what else to put. Can you help me?

Hi @CBID2, thanks for creating and linking a PR. I'll give it a review and any follow up comments there, but from a quick look I think you took care of everything that's needed ✨.

This issue thread got a bit confusing since we're discussing both the style guide and another file.

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 contributing docs Content related to our contributing docs help wanted Anyone is welcome to open a pull request to fix this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants
@janbrasna @felicitymay @ethanpalm @CBID2 @nguyenalex836 @gsosm0760 and others