jkorous marked 4 inline comments as done.
jkorous added a comment.

Updated the patch.



================
Comment at: unittests/Basic/FileManagerTest.cpp:349
 
+// rdar://problem/47536127
+TEST_F(FileManagerTest, getFileDontOpenRealPath) {
----------------
arphaman wrote:
> jkorous wrote:
> > arphaman wrote:
> > > Please leave this comment out
> > Sure, no problem.
> > 
> > Just so I understand it - when it is appropriate to quote the radar? I see 
> > a lot of them in clang/test. Are these just from the "ancient" history?
> There are some radars but that doesn't mean we should still be adding them. 
> They don't provide any context or value for the developers here, especially 
> in a comment. For a unit test like this the name should generally provide 
> some form of self-contained description of the scenario, which it does here. 
> I would recommend leaving the radar number in a commit message instead.
Got it. Thanks!


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

https://reviews.llvm.org/D58213



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

Reply via email to