benshi001 marked 2 inline comments as done. benshi001 added inline comments.
================ Comment at: clang/lib/Driver/ToolChains/AVR.cpp:356 +void AVRToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs, + ArgStringList &CC1Args) const { ---------------- Anastasia wrote: > benshi001 wrote: > > Anastasia wrote: > > > benshi001 wrote: > > > > Anastasia wrote: > > > > > function name doesn't adhere to the coding style. > > > > Not sure your concern ? Other platforms also use the same function name. > > > > > > > > ``` > > > > void RISCVToolChain::AddClangSystemIncludeArgs(const ArgList > > > > &DriverArgs, > > > > ArgStringList &CC1Args) > > > > const { > > > > void Linux::AddClangSystemIncludeArgs(const ArgList &DriverArgs, > > > > ArgStringList &CC1Args) const { > > > > void WebAssembly::AddClangSystemIncludeArgs(const ArgList &DriverArgs, > > > > ArgStringList &CC1Args) > > > > const { > > > > ``` > > > https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly > > > > > > > > > > > > > Function names should be verb phrases (as they represent actions), and > > > > command-like function should be imperative. The name should be camel > > > > case, and start with a lower case letter (e.g. openFile() or isFoo()). > > > > > > > > > You should adhere to the coding style in the new functionality. > > > > > I have created another patch for that, and would like to keep current patch > > simple without noise. > > > > https://reviews.llvm.org/D100022 > Sorry, my mistake - I haven't realized that you are overriding the method > instead of adding a new one. Then I think it is best to keep the old style > because we don't normally do purely formatting/renaming changes. Thanks. You are appreciated to go on with this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97669/new/ https://reviews.llvm.org/D97669 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits