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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits