balazske added a comment.

It looks mostly good now, apart from the line 1 comment problem. Usually in a 
review all phabricator diffs are created against the original commit or at 
least against an existing (in the current github repository) commit, here the 
last diff seems to be against an other commit. The next diff should be based on 
an existing commit (can be same as the original base). Otherwise the `arc 
patch` command may not work (I do not know if you use it) and it is difficult 
to compare changes related to the original code.



================
Comment at: clang/include/clang/AST/ASTImportError.h:2
+//===- ASTImportError.h -  Define errors while importing AST  -------*- C++
+//-*-===//
 //
----------------
One small problem: This line 2 is the end of line 1, this should be part of 
line 1. Line 1 should be exactly 80 characters long.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124774

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

Reply via email to