arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land.
In https://reviews.llvm.org/D40563#940536, @ioeric wrote: > In https://reviews.llvm.org/D40563#939964, @arphaman wrote: > > > If nothing uses `getCXXScopeSpecifier` right now we can't really test it > > with a clang or c-index-test regression test. A completion unit test could > > work here. I don't think we actually have existing completion unit tests > > though, so you would have to create one from scratch. But if > > `getCXXScopeSpecifier` will be used in a follow up patch maybe it will be > > easier to commit this without a test together with the followup patch? > > > I have another clangd patch that uses this, but this would still need to be a > separate patch since they are in different repos... You can commit the two in one either using the monrepo or sparse SVN checkout ;) But I wouldn't object if you commit this separately right before the clangd patch, so LGTM https://reviews.llvm.org/D40563 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
