[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header and rename it .

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.


================
Comment at: clang/include/clang/AST/ASTImportError.h:1
+//===- ASTImporter.h - Import Error while Importing AST -------*- C++ -*-===//
+//
----------------
balazske wrote:
> 



================
Comment at: clang/include/clang/AST/ASTImportError.h:22
+
+class ImportError : public llvm::ErrorInfo<ImportError> {
+public:
----------------
phyBrackets wrote:
> balazske wrote:
> > balazske wrote:
> > > Rename to `ASTImportError`.
> > The rename to `ASTImportError` is better in a separate patch. The rename would touch code in much more places and there is a rule to make independent changes in separate patches. Still I think this class should not be called just `ImportError` if it has an own header (it is not an internal-like class of `ASTImporter` as is used to be).
> So, what do you suggest , for this patch should I go with name //ImportError// only ? And rename it as //ASTImportError// in a separate patch ?
Yes: Do not change the class name here (but header should be called ASTImportError.h). In a next patch make only a rename.


================
Comment at: clang/include/clang/AST/ASTImportError.h:52
+#endif // LLVM_CLANG_AST_ASTIMPORTERROR_H
\ No newline at end of file

----------------
Please add newline. I do not know if this can cause buildbot failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124774