awarzynski requested changes to this revision. awarzynski added a comment. This revision now requires changes to proceed.
Thank you for submitting this @achieveartificialintelligence ! The aim of these changes is to improve the consistency of the code-base with respect to the coding standards documented separately for Clang <https://llvm.org/docs/CodingStandards.html#llvm-coding-standards> and Flang <https://github.com/llvm/llvm-project/blob/main/flang/docs/C%2B%2Bstyle.md>. I think that this is a very positive suggestion, but would like to request a few refinements before this is ready: - Please note that in LLVM/Clang, variables names start with upper case (so it should be int `main(int Argc, const char **Argv)` instead of `main(int argc, const char **argv)` ). link <https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly> - In Flang the rules are slightly different, and the variable names start with a lower case (so it should be int `main(int Argc, const char **Argv)` instead of `main(int argc, const char **argv)` ). link <https://github.com/llvm/llvm-project/blob/main/flang/docs/C%2B%2Bstyle.md#naming> - Your commit message refers to _a driver_, but in fact you are updating 2 specific drivers: Clang compiler driver and Flang compiler driver - could you clarify? - Your commit message refers to `argc_` specifically, but you are updating `argc_`, `argv_` and `Argv/argv` too. Personally I'd prefer `ArgC/argC` and `ArgV/argV` for input parameters and `ArgValues/argValues` for the local variable. This way it would be more self-explanatory. But that's a matter of style. As long as this is consistent with the coding standards, I think that this should be accepted. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97138/new/ https://reviews.llvm.org/D97138 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
