zturner added a comment. In https://reviews.llvm.org/D23409#513017, @alexfh wrote:
> May I ask you to upload patches with full diff context next time? I know, > it's not directly supported by TortoiseGit, but there are at least two other > reasonable ways of generating full diffs for Phabricator: > http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface > and > http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-command-line. > > Wrt testing, I think, apart from a new clang-tidy test, the changes to Driver > and Tooling need separate tests as well. I'm unsure what to actually test in the Driver and Tooling tests. For example, in Tooling we just change from using one function to using another function. And that function is already tested in the LLVM test suite. It seems redundant to add another test that is for all intents and purposes equivalent to "Does LLVM's windows command line parser work correctly?", which is already tested. How necessary is this? For the clang-tidy test, I will need to make it only enabled on Windows, since other platforms are not going to be building clang-cl. I think I should be able to figure that out. https://reviews.llvm.org/D23409 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
