[PATCH] D95018: [NFC][AMDGPU] Document target ID syntax for code object V2 to V3

[Scott Linder via Phabricator via llvm-commits]

scott.linder added a comment.

In D95018#2523019 <https://reviews.llvm.org/D95018#2523019>, @MaskRay wrote:

> In your commit the message does not include `Reviewed by:`. Many people agree that both `Reviewed by:` & `Differential Revision:` should be present. The `Reviewed by:` list indicates people who acknowledged the patch. (The `Reviewers:` list does not necessarily mean all the people on the list have acknowledged the patch so `Reviewers:` is mostly useless.)
>
> `arc amend` can fetch the Phabricator summary and amend the local description.
>
> You can install `llvm/utils/git/pre-push.py` to prevent accidental `Summary:`, `Reviewers:`, `Subscribers:` and `Tags:` in the presence of `Differential Revision:`.

Why is including anything besides "Differential Revision:" being permitted/required? It seems unfortunate enough that anything is needed in the commit message at all, but at least it is a single, relatively unobtrusive thing. "Denormalizing" the message by duplicating information contained in the Differential Revision doesn't seem helpful.

If the utility is in avoiding the need to query Phabricator to get this information, then can we make the "Reviewed By:" field include full names, rather than (often opaque) usernames/handles?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95018/new/

https://reviews.llvm.org/D95018