ioeric added inline comments.
================ Comment at: clangd/URI.h:31 +public: + /// \brief Returns decoded scheme e.g. "https" + llvm::StringRef scheme() const { return Scheme; } ---------------- sammccall wrote: > nit: prefer to omit `\brief` unless you want the brief comment to be > something other than the first sentence. > > (I know we have this in lots of places - it adds noise, and I finally got > around to looking it up in the style guide!) Good to know. Thanks! ================ Comment at: unittests/clangd/URITests.cpp:1 +//===-- URITests.cpp ---------------------------------*- C++ -*-----------===// +// ---------------- sammccall wrote: > you may want a test that round-trips `getVirtualTestFilePath("x")` through a > file URI and back again. > This should exercise the platform-specific code path - that path is under C:\ > on windows, and /tmp/ on linux. The buildbots should give us platform > coverage. Good idea! Didn't know such a nice function exists. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41946 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits