This is an archive of the discontinued LLVM Phabricator instance.

Differential D131500

[Docs][CodeReview] Add a small paragraph on adding tokens, NFC.
ClosedPublic

Authored by bzcheeseman on Aug 9 2022, 8:11 AM.

Details

Reviewers
whisperity
Commits
rG688178aa2bd3: [Docs][CodeReview] Add a small paragraph on adding tokens, NFC.

Diff Detail

Event Timeline

bzcheeseman created this revision.Aug 9 2022, 8:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2022, 8:11 AM
bzcheeseman requested review of this revision.Aug 9 2022, 8:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2022, 8:11 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript
bzcheeseman added a reviewer: whisperity.Aug 9 2022, 8:12 AM
Herald added a subscriber: rnkovacs. · View Herald TranscriptAug 9 2022, 8:12 AM
bzcheeseman added a reviewer: lattner.Aug 9 2022, 8:12 AM
bzcheeseman removed a reviewer: lattner.
Harbormaster completed remote builds in B180178: Diff 451161.Aug 9 2022, 11:33 AM
whisperity accepted this revision.Aug 30 2022, 1:43 AM

Hey!

Sorry, I'm not sure how I missed it. Been on vacations and such... either way, I think documenting this is a good practice.

Do you have commit access or shall I commit on your behalf? If so, what do you want to have as the author name and e-mail for the commit?

llvm/docs/CodeReview.rst
151

Let's use the actual text verbatim from the GUI? 🙂

This revision is now accepted and ready to land.Aug 30 2022, 1:43 AM
bzcheeseman added a comment.Aug 30 2022, 4:27 PM
In D131500#3757581, @whisperity wrote:

Hey!

Sorry, I'm not sure how I missed it. Been on vacations and such... either way, I think documenting this is a good practice.

Do you have commit access or shall I commit on your behalf? If so, what do you want to have as the author name and e-mail for the commit?

I have commit access, I'll make the change you suggested and push it in.

bzcheeseman updated this revision to Diff 456819.Aug 30 2022, 4:59 PM
bzcheeseman marked an inline comment as done.Aug 30 2022, 5:00 PM
This revision was landed with ongoing or failed builds.Aug 30 2022, 5:01 PM
Closed by commit rG688178aa2bd3: [Docs][CodeReview] Add a small paragraph on adding tokens, NFC. (authored by bzcheeseman). · Explain Why
This revision was automatically updated to reflect the committed changes.
bzcheeseman added a commit: rG688178aa2bd3: [Docs][CodeReview] Add a small paragraph on adding tokens, NFC..
Harbormaster completed remote builds in B184260: Diff 456819.Aug 30 2022, 6:17 PM

Revision Contents

PathSize
llvm/
docs/
6 lines

Diff 456820

llvm/docs/CodeReview.rst

Show First 20 Lines • Show All 141 Lines • ▼ Show 20 Lines
responsibility of the reviewer to have reviewed all of the discussion and responsibility of the reviewer to have reviewed all of the discussion and
feedback from all reviewers ensuring that all feedback has been addressed and feedback from all reviewers ensuring that all feedback has been addressed and
that all other reviewers will almost surely be satisfied with the patch being that all other reviewers will almost surely be satisfied with the patch being
approved. If unsure, the reviewer should provide a qualified approval, (e.g., approved. If unsure, the reviewer should provide a qualified approval, (e.g.,
"LGTM, but please wait for @someone, @someone_else"). You may also do this if "LGTM, but please wait for @someone, @someone_else"). You may also do this if
you are fairly certain that a particular community member will wish to review, you are fairly certain that a particular community member will wish to review,
even if that person hasn't done so yet. even if that person hasn't done so yet.
If you approve of the idea/concept of a patch but feel unqualified to approve,
another option (other than accepting the patch) is to simply *"Award Token"*
whisperityUnsubmitted
Done
ReplyInline Actions
If you approve of the idea/concept of a patch but feel unqualified to approve,
- another option (other than accepting the patch) is to simply award a token
- (right-hand side in Phabricator) to indicate support without indicating to
+ another option (other than accepting the patch) is to simply *"Award Token"*(right-hand side in Phabricator) to indicate support without indicating to

Let's use the actual text verbatim from the GUI? 🙂

whisperity: Let's use the actual text verbatim from the GUI? 🙂
(right-hand side in Phabricator) to indicate support without indicating to
other reviewers that the patch has been accepted and reviewed in their
dashboard.
Note that, if a reviewer has requested a particular community member to review, Note that, if a reviewer has requested a particular community member to review,
and after a week that community member has yet to respond, feel free to ping and after a week that community member has yet to respond, feel free to ping
the patch (which literally means submitting a comment on the patch with the the patch (which literally means submitting a comment on the patch with the
word, "Ping."), or alternatively, ask the original reviewer for further word, "Ping."), or alternatively, ask the original reviewer for further
suggestions. suggestions.
If it is likely that others will want to review a recently-posted patch, If it is likely that others will want to review a recently-posted patch,
especially if there might be objections, but no one else has done so yet, it is especially if there might be objections, but no one else has done so yet, it is
▲ Show 20 Lines • Show All 94 Lines • Show Last 20 Lines