[PATCH] D95079: [NFC] Move ImportedFunctionsInliningStatistics to Analysis

Tue Jan 26 10:30:10 PST 2021

dblaikie added a comment.

In D95079#2523023 <https://reviews.llvm.org/D95079#2523023>, @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:`.

FWIW, my take on it would be that including "Reviewed by:" is not harmful/may be useful (as opposed to the other fields (except Differential Revision), which I think actively add noise to the commit messages), but I wouldn't say it's required/don't think it rises to the level of needing to be fixed/corrected.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95079