[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

[Don Hinton via Phabricator via cfe-commits]

hintonda abandoned this revision.
hintonda added a comment.

Replaced by D60629 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D60151#1463484 , @MyDeveloperDay 
wrote:

> >> I suppose we could keep the names and directory structure and just change 
> >> the namespace.  That would just be a special case in the scripts.  Haven't 
> >> looked into it yet, but will do so as soon as I can.
> > 
> > Isn't that matching done on strings? I.e. is there difference between 
> > `-llvm-*` and `-ll*` ?
>
> it would be if they only do it to -llm-* (which could possibly catch most 
> cases)
>
> but what if someone is doing
>
> -llvm-header-guard
>
> We wouldn't catch those..
>
> This is a snippet of a .clang-tidy file from a  project I work on, we turn 
> off specific checks by fully qualifying the checker
>
>   
>   llvm-*,
>   -llvm-header-guard,
>   -llvm-include-order,
>   misc-*,
>   modernize-*,
>   ...
>


You make a great point.  I'll look into just changing the namespace name.  I 
wasn't really comfortable with all the code churn it involved anyway, but once 
I got started, I figured I'd finish it.  The duplicate `llvm` namespace is the 
issue, not other names.  Thanks for the feedback...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.



>> I suppose we could keep the names and directory structure and just change 
>> the namespace.  That would just be a special case in the scripts.  Haven't 
>> looked into it yet, but will do so as soon as I can.
> 
> Isn't that matching done on strings? I.e. is there difference between 
> `-llvm-*` and `-ll*` ?

it would be if they only do it to -llm-* (which could possibly catch most cases)

but what if someone is doing

-llvm-header-guard

We wouldn't catch those..

This is a snippet of a .clang-tidy file from a  project I work on, we turn off 
specific checks by fully qualifying the checker

  
  llvm-*,
  -llvm-header-guard,
  -llvm-include-order,
  misc-*,
  modernize-*,
  ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D60151#1462976 , @hintonda wrote:

> In D60151#1462850 , @MyDeveloperDay 
> wrote:
>
> > are we supporting "-llvm-*" in existing .clang-tidy files?
> >
> > If people selectively turn checkers off, won't all of a sudden everyone 
> > start getting llvm-project checks and fixes turned back on?
> >
> > https://github.com/search?q=-llvm-%2A=Code
> >
> > maybe we need to add something like:
> >
> >   llvm-header-guard (redirects to llvm-project-header-guard) 
> > 
> >   llvm-header-include-order (redirects to llvm-project-include-order) 
> > 
> >   llvm-header-namespace-comment(redirects to 
> > llvm-project-namespace-comment) 
> >   llvm-header-twine-local(redirects to llvm-project-twine-local) 
> > 
> >  
> >
>
>
> I suppose we could keep the names and directory structure and just change the 
> namespace.  That would just be a special case in the scripts.  Haven't looked 
> into it yet, but will do so as soon as I can.


Isn't that matching done on strings? I.e. is there difference between `-llvm-*` 
and `-ll*` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D60151#1462850 , @MyDeveloperDay 
wrote:

> are we supporting "-llvm-*" in existing .clang-tidy files?
>
> If people selectively turn checkers off, won't all of a sudden everyone start 
> getting llvm-project checks and fixes turned back on?
>
> https://github.com/search?q=-llvm-%2A=Code
>
> maybe we need to add something like:
>
>   llvm-header-guard (redirects to llvm-project-header-guard) 
> 
>   llvm-header-include-order (redirects to llvm-project-include-order) 
> 
>   llvm-header-namespace-comment(redirects to llvm-project-namespace-comment) 
> 
>   llvm-header-twine-local(redirects to llvm-project-twine-local) 
> 
>  
>


I suppose we could keep the names and directory structure and just change the 
namespace.  That would just be a special case in the scripts.  Haven't looked 
into it yet, but will do so as soon as I can.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

are we supporting "-llvm-*" in existing .clang-tidy files?

If people selectively turn checkers off, won't all of a sudden everyone start 
getting llvm-project checks and fixes turned back on?

https://github.com/search?q=-llvm-%2A=Code

maybe we need to add something like:

  llvm-header-guard (redirects to llvm-project-header-guard) 
  llvm-header-include-order (redirects to llvm-project-include-order) 

  llvm-header-namespace-comment(redirects to llvm-project-namespace-comment) 

  llvm-header-twine-local(redirects to llvm-project-twine-local) 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D60151#1454802 , @hintonda wrote:

> In D60151#1454741 , @alexfh wrote:
>
> > In D60151#1454105 , @hintonda 
> > wrote:
> >
> > > - Rename llvm directory to llvm_project.
> > > - Change llvm- to llvm-project-.
> > > - Rename files.
> >
> >
> > Awesome! Thanks for doing this. Could you ensure that the add_new_check.py 
> > script still works?
>
>
> Thanks for mentioning this.  Found a missing rename discovered by 
> add_new_check.py which I'll checkin shortly.  However, I'm not sure how best 
> to solve a rename issue.
>
> In rename_check.py, module names can't be hyphenated, e.g.:
>
>   190:  old_module = args.old_check_name.split('-')[0]
>   191:  new_module = args.new_check_name.split('-')[0]
>   
>
> If we keep this pattern, then "llvm-project" won't work.  It would need to be 
> something like "llvmproject" or something else.  Please let me know your 
> preference.


Hmm, that's unfortunate behavior. I guess my preference would be to go with 
`llvm_project`, but it's not a strong preference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D60151#1454741 , @alexfh wrote:

> In D60151#1454105 , @hintonda wrote:
>
> > - Rename llvm directory to llvm_project.
> > - Change llvm- to llvm-project-.
> > - Rename files.
>
>
> Awesome! Thanks for doing this. Could you ensure that the add_new_check.py 
> script still works?


Thanks for mentioning this.  Found a missing rename discovered by 
add_new_check.py which I'll checkin shortly.  However, I'm not sure how best to 
solve a rename issue.

In rename_check.py, module names can't be hyphenated, e.g.:

  190:  old_module = args.old_check_name.split('-')[0]
  191:  new_module = args.new_check_name.split('-')[0]

If we keep this pattern, then "llvm-project" won't work.  It would need to be 
something like "llvmproject" or something else.  Please let me know your 
preference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In D60151#1454105 , @hintonda wrote:

> - Rename llvm directory to llvm_project.
> - Change llvm- to llvm-project-.
> - Rename files.


Awesome! Thanks for doing this. Could you ensure that the add_new_check.py 
script still works?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60151



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits