[PATCH] D101529: Fix `-Wsuggest-override` warning when building benchmark with clang-cl on Windows

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 1 00:40:29 PDT 2021


lebedev.ri added a comment.

In D101529#2852271 <https://reviews.llvm.org/D101529#2852271>, @kbobyrev wrote:

> In D101529#2847820 <https://reviews.llvm.org/D101529#2847820>, @SibiSiddharthan wrote:
>
>> In D101529#2843405 <https://reviews.llvm.org/D101529#2843405>, @kbobyrev wrote:
>>
>>> Hey! If you're still interested in landing it, please make sure to land it into the upstream first and mention the revision here.
>>
>> Hey, I have a question. When you say `upstream` do you mean here https://github.com/google/benchmark ?
>
> Correct, upstream means the main benchmark repo.
>
>> If so, do I have to raise a pull request there and reference it here?.
>
> The process looks as follows: you can open a PR in there, have it approved and landed in the main branch. Then you have the https://github.com/google/benchmark/commit/<YOUR_COMMIT_HASH> and you insert it into the Phabricator revision description, also mentioning it in the file where we have all the modifications.
>
>> I do see that the benchmark under the llvm-project has changes that are not present in https://github.com/google/benchmark
>
> Yeah, this is unfortunately true but it isn't really a good practice :( If you open a PR and nobody looks into that and it doesn't get merged reasonably fast then it'd be OK to just land the change here, but I would be happy if you could open a PR. If there won't be any response from the maintainers in several days, we'll just land this here.

FWIW i think all such PR's have been merged within hours of being opened.
Tides may change once said library starts depending on abseil though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101529



More information about the llvm-commits mailing list