alexfh added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/add_new_check.py:382
+ if module == 'llvm' or module == 'clang':
+ namespace = module + '_checker'
+ else:
----------------
hintonda wrote:
> aaron.ballman wrote:
> > I thought we were going with `llvm_check`?
> Just typo on my part. I the options mentioned before where things like
> `llvm_project` or `llvm_proj`, etc. I just settled on `llvm_check` because
> it sounded better -- then actually typed `checker`.
>
> I'll fix it this afternoon.
I have another suggestion re: the color of this bike shed, namely:
`llvm_checks`. But I don't feel strongly about this.
================
Comment at: clang-tools-extra/clang-tidy/add_new_check.py:381
+
+ # Don't allow 'clang' or 'llvm' namespaces
+ if module == 'llvm':
----------------
I'd add an explanation: `# Map module names to namespace names that don't
conflict with widely used top-level namespaces.`
And again, no need to mention `clang` here.
================
Comment at: clang-tools-extra/clang-tidy/rename_check.py:218
header_guard_variants = [
+ (args.old_check_name.replace('-', '_') + '_Check').upper(),
+ (old_module + '_' + check_name_camel).upper(),
----------------
s/_Check/_CHECK/, maybe?
================
Comment at: clang-tools-extra/clang-tidy/rename_check.py:267
- if old_module != new_module:
+ if old_module != new_module or new_module == 'llvm':
+ if new_module == 'llvm':
----------------
I believe, we should first construct the new namespace name (using exactly the
same code as in add_new_check.py) and then check if it differs from the old one.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60629/new/
https://reviews.llvm.org/D60629
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
