kbobyrev accepted this revision.
kbobyrev added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:257
+
+ public:
+ TextProto(const google::protobuf::Message &M) : M(M) {}
----------------
nit: everything is public by default since it's `struct`, right? did you mean
to put `&M` behind `private`?
================
Comment at: clang-tools-extra/clangd/test/remote-index/pipeline_helper.py:25
-
def main():
----------------
nit: PEP is against having <2 lines between functions :(
================
Comment at: clang-tools-extra/clangd/test/remote-index/pipeline_helper.py:31
+ parser.add_argument('--server-arg', action='append')
+ parser.add_argument('--server-log', nargs='?', type=argparse.FileType('wb'),
default=os.devnull)
----------------
Why do we need `wb` here instead of `w`? I know that `subprocess.Popen` writes
binary data but since we're writing actual text to file maybe we could convert
to non-binary?
================
Comment at: clang-tools-extra/clangd/test/remote-index/pipeline_helper.py:81
if __name__ == '__main__':
main()
----------------
this was `pipeline_helper.py` intended for `pipeline.test`, maybe we'll use it
more so makes sense to rename it to just `helper.py` or somethting.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90654/new/
https://reviews.llvm.org/D90654
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
