docs icon indicating copy to clipboard operation
docs copied to clipboard

`Requesting a pull request review` needs a review

Open jsoref opened this issue 2 years ago • 9 comments

Code of Conduct

What article on docs.github.com is affected?

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/requesting-a-pull-request-review

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

-If you're an organization member, you can also request a specific team to review your changes.
+If you're an organization member, you can also request a specific team to review the PR.

It's quite common for one member of an org to request reviews on a non-member contributed PR from other members of the org.


The ability to add multiple pull request reviewers or requests reviews from teams is available in public repositories with GitHub Free for organizations and legacy per-repository billing plans, and in public and private repositories with GitHub Team, GitHub Enterprise Server, and GitHub Enterprise Cloud. For more information, see "GitHub's products."

At this point, I suspect it'd be easier to enumerate where it isn't available. Is the answer:

The ability to add multiple pull request reviewers or requests reviews from teams is not available in private repositories with GitHub Free accounts.

I understand that the code for that comes from:

{% ifversion fpt or ghae or ghes or ghec %}

But, I wonder if there's support for negation in that...


Repositories belong to a personal account (a single individual owner) or an organization account (a shared account with numerous collaborators or maintainers).

Dropping the parentheses, that reads:

Repositories belong to a personal account or an organization account.

That feels like a truism... I'm not sure what it's trying to say.

Owners and collaborators on a repository owned by a personal account can assign pull request reviews. Organization members with triage permissions can also assign a reviewer for a pull request.

The also here is problematic. The personal account and organization account cases are mutually exclusive. Handling them in the same paragraph as here is confusing and misleading.

To assign a reviewer to a pull request, you will need write access to the repository.

This is kinda misleading. Sometimes the act of creating a pull request will automatically assign a reviewer. I've tripped on this a number of times recently. Yes, it's true, I don't have the ability to directly target review requests using the widget described at the bottom of the page, but based on which files I change, a CODEOWNERS file may direct review requests to specific accounts/teams. (In my case, I changed too many files and triggered review requests to too many teams which frustrated various people.) For the record, I don't have write access to the relevant repositories (I do have read access...).

Yes, it's a really edgy edge. But, I'm noting it here because it should be noted somewhere, and if someone reads the changed document and it isn't mentioned, they might read the linked issue and discover this tidbit. (And if it is mentioned, then, great.)

One way to address this concern is to use the word manually or interactively in places, e.g. Requesting a pull request review manually.

For more information about repository access, see "Repository roles for an organization." If you have write access, you can assign anyone who has read access to the repository as a reviewer.

The write access part here contradicts the triage role statement in the preceding paragraph. Or rather, if there's meaning to the distinction, it's easily missed. Is it trying to say:

Users with triage can only ask for reviews from accounts, whereas users with write can also ask for reviews from teams.

If so, that'd be clearer.

Note that the word assign should not be used in this document as it conflates review requests with Assigning issues and pull requests to other GitHub users which is an entirely separate action. Sadly, these are already confused by end users, and documentation should not make further such confusion.

I'd suggest splitting the paragraph into two, one for repositories owned by personal accounts and one for repositories owned by organization accounts.

I occasionally run into enterprise accounts and theoretically Enterprise Managed User accounts, but don't know if they can own repositories outside of organizations. If they can, it might be worth indicating which way they fall. If they can't, I'm not sure if it's worth noting they can't host repositories and thus aren't relevant to this document.


Note: Pull request authors can't request reviews unless they are either a repository owner or collaborator with write access to the repository.

What if I only have Triage permission?

Note that Repository roles for organizations only talks about outside collaborators plus Read, Triage, Write, Maintain, and Admin which implies that a collaborator is none of them (and thus not Triage).

Note that collaborators are a thing in personal repositories -- see Inviting collaborators to a personal repository. They were explicitly mentioned earlier in this document. So there's no reason to assume collaborators means org member.


You can request a review from either a suggested or specific person.

I'd suggest using account instead of person -- possibly personal account. Persons aren't things in github, there are really only accounts. (This also applies to the very first sentence in the doc.) As a simple example, afaik @octocat is not a person.


Once someone has reviewed your pull request and you've made the necessary changes, you can re-request review from the same reviewer.

Speaking from personal experience, this is false. If someone else re-requests a review from that reviewer, you can't re-request review again until they make a new review. This was incredibly confusing.


If the requested reviewer does not submit a review, and the pull request meets the repository's mergeability requirements, you can still merge the pull request.

This is false. If I have Triage but not Write, I can't merge a PR even though I could request review.


Under your repository name, click Pull requests.

your -> the


Optionally, if you know the name of the person or team you'd like a review from, click Reviewers, then type the username of the person or the name of the team you're asking to review your changes. Click their team name or username to request a review.

Caveat about "do you need extra permissions to ask for a review from a team"?

Additional information

No response

jsoref avatar Jul 11 '22 23:07 jsoref

Code of Conduct

What article on docs.github.com is affected?

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/requesting-a-pull-request-review

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

-If you're an organization member, you can also request a specific team to review your changes.
+If you're an organization member, you can also request a specific team to review the PR.

It's quite common for one member of an org to request reviews on a non-member contributed PR from other members of the org.

The ability to add multiple pull request reviewers or requests reviews from teams is available in public repositories with GitHub Free for organizations and legacy per-repository billing plans, and in public and private repositories with GitHub Team, GitHub Enterprise Server, and GitHub Enterprise Cloud. For more information, see "GitHub's products."

At this point, I suspect it'd be easier to enumerate where it isn't available. Is the answer:

The ability to add multiple pull request reviewers or requests reviews from teams is not available in private repositories with GitHub Free accounts.

I understand that the code for that comes from:

{% ifversion fpt or ghae or ghes or ghec %}

But, I wonder if there's support for negation in that...

Repositories belong to a personal account (a single individual owner) or an organization account (a shared account with numerous collaborators or maintainers).

Dropping the parentheses, that reads:

Repositories belong to a personal account or an organization account.

That feels like a truism... I'm not sure what it's trying to say.

Owners and collaborators on a repository owned by a personal account can assign pull request reviews. Organization members with triage permissions can also assign a reviewer for a pull request.

The also here is problematic. The personal account and organization account cases are mutually exclusive. Handling them in the same paragraph as here is confusing and misleading.

To assign a reviewer to a pull request, you will need write access to the repository.

This is kinda misleading. Sometimes the act of creating a pull request will automatically assign a reviewer. I've tripped on this a number of times recently. Yes, it's true, I don't have the ability to directly target review requests using the widget described at the bottom of the page, but based on which files I change, a CODEOWNERS file may direct review requests to specific accounts/teams. (In my case, I changed too many files and triggered review requests to too many teams which frustrated various people.) For the record, I don't have write access to the relevant repositories (I do have read access...).

Yes, it's a really edgy edge. But, I'm noting it here because it should be noted somewhere, and if someone reads the changed document and it isn't mentioned, they might read the linked issue and discover this tidbit. (And if it is mentioned, then, great.)

One way to address this concern is to use the word manually or interactively in places, e.g. Requesting a pull request review manually.

For more information about repository access, see "Repository roles for an organization." If you have write access, you can assign anyone who has read access to the repository as a reviewer.

The write access part here contradicts the triage role statement in the preceding paragraph. Or rather, if there's meaning to the distinction, it's easily missed. Is it trying to say:

Users with triage can only ask for reviews from accounts, whereas users with write can also ask for reviews from teams.

If so, that'd be clearer.

Note that the word assign should not be used in this document as it conflates review requests with Assigning issues and pull requests to other GitHub users which is an entirely separate action. Sadly, these are already confused by end users, and documentation should not make further such confusion.

I'd suggest splitting the paragraph into two, one for repositories owned by personal accounts and one for repositories owned by organization accounts.

I occasionally run into enterprise accounts and theoretically Enterprise Managed User accounts, but don't know if they can own repositories outside of organizations. If they can, it might be worth indicating which way they fall. If they can't, I'm not sure if it's worth noting they can't host repositories and thus aren't relevant to this document.

Note: Pull request authors can't request reviews unless they are either a repository owner or collaborator with write access to the repository.

What if I only have Triage permission?

Note that Repository roles for organizations only talks about outside collaborators plus Read, Triage, Write, Maintain, and Admin which implies that a collaborator is none of them (and thus not Triage).

Note that collaborators are a thing in personal repositories -- see Inviting collaborators to a personal repository. They were explicitly mentioned earlier in this document. So there's no reason to assume collaborators means org member.

You can request a review from either a suggested or specific person.

I'd suggest using account instead of person -- possibly personal account. Persons aren't things in github, there are really only accounts. (This also applies to the very first sentence in the doc.) As a simple example, afaik @octocat is not a person.

Once someone has reviewed your pull request and you've made the necessary changes, you can re-request review from the same reviewer.

Speaking from personal experience, this is false. If someone else re-requests a review from that reviewer, you can't re-request review again until they make a new review. This was incredibly confusing.

If the requested reviewer does not submit a review, and the pull request meets the repository's mergeability requirements, you can still merge the pull request.

This is false. If I have Triage but not Write, I can't merge a PR even though I could request review.

Under your repository name, click Pull requests.

your -> the

Optionally, if you know the name of the person or team you'd like a review from, click Reviewers, then type the username of the person or the name of the team you're asking to review your changes. Click their team name or username to request a review.

Caveat about "do you need extra permissions to ask for a review from a team"?

Additional information

No response

commid

HOALND avatar Jul 11 '22 23:07 HOALND

@jsoref Thanks for opening an issue and providing all this information 💖. I'll get this triaged for review! ⚡

cmwilson21 avatar Jul 12 '22 23:07 cmwilson21

All help can come from everyone of us that makes us all team together

Benson665 avatar Jul 13 '22 00:07 Benson665

Can I work on this issue?

nawed2611 avatar Jul 15 '22 20:07 nawed2611

I personally wouldn't until I got a response from GitHub.

jsoref avatar Jul 15 '22 20:07 jsoref

@nawed2611 Thanks for your interest in this issue! Right now, it's still waiting for review.

For issues that are open for anyone, check out our help wanted section to find open issues you can work on. 💛

cmwilson21 avatar Jul 15 '22 20:07 cmwilson21

@nawed2611 Thanks for your interest in this issue! Right now, it's still waiting for review.

For issues that are open for anyone, check out our help wanted section to find open issues you can work on. 💛

Sure Thanks!

nawed2611 avatar Jul 16 '22 06:07 nawed2611

yes Visit Our Site Or Call For A Free Consultation (636) 385 - 4494 https://frank-andrews-radio-media-advertisement.business.site

https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=icon Virus-free. www.avast.com https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=link <#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>

On Fri, Jul 15, 2022 at 3:13 PM Nawed Ali @.***> wrote:

Can I work on this issue?

— Reply to this email directly, view it on GitHub https://github.com/github/docs/issues/18974#issuecomment-1185874951, or unsubscribe https://github.com/notifications/unsubscribe-auth/A2B5ZOCBJRGUB4VUORY35ZTVUHA4XANCNFSM53JDMCQQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

Andrewsmedia avatar Jul 17 '22 08:07 Andrewsmedia

tq

HOALND avatar Jul 19 '22 00:07 HOALND

Thanks for letting us know about this @jsoref! You're right, this is a gnarly area of the product with lots of tiny nuances. We've opened an internal issue to collaborate more closely with some other teams as we try to figure out what the best updates might be.

Thanks again!

janiceilene avatar Sep 13 '22 06:09 janiceilene