hokein added a comment.

the code looks good to me, but we need to be a bit careful on landing this -- 
as we have an internal client setting this flag.



================
Comment at: clang-tools-extra/clangd/ClangdServer.h:147
 
-    bool SuggestMissingIncludes = false;
-
----------------
our internal client explicitly set this to `true`, so we need a migration plan 
for this, otherwise this would break our build of internal client during the 
integration, a possible plan is

1. set this flag to true by default in upstream, wait for the integration
2. remove the explicit setting internally
3. remove this flag (this patch) in upstream

Or just remove the flag internally, then land this patch in upstream (but 
internal release has to pick-up these two together)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94727

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

Reply via email to