[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