aaron.ballman added a comment.

In D98635#2626485 <https://reviews.llvm.org/D98635#2626485>, @whisperity wrote:

> In D98635#2626464 <https://reviews.llvm.org/D98635#2626464>, @whisperity 
> wrote:
>
>> Strange because I specifically ran both `check-clang` **and** 
>> `check-clang-tools` locally, but will look into this.
>
> Turns out there is a `check-clang-extra-unit` too, and it looks like those 
> test TUs either didn't compile for me at all (as in, it was not commanded to 
> compile them), or I messed something up during the rebase.
> (But TIL we finally have some semblance of pre-merge checking instead of 
> always reverting commits! 😋)

Yes, it's actually been catching some useful things lately, too, which is nice!

> What's the outlook on the executive decision... changing the **schema** in 
> `libToolingCore`... is such even allowed? I'm venturing into parts unknown 
> here.

I think @alexfh will have to make the final call, but I think the changes here 
look reasonable. I have no idea if we've made stability guarantees about the 
schema though.

I'm giving my LGTM, but not accepting the review because I'd like to hear from 
Alex before this lands.


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

https://reviews.llvm.org/D98635

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to