carlosgalvezp added a comment.
Thanks for the fix! Some minor comments/questions.
================
Comment at: clang-tools-extra/clang-tidy/rename_check.py:75
print("Renaming '%s' -> '%s'..." % (fileName, newFileName))
- os.rename(fileName, newFileName)
+ subprocess.check_call(["git", "mv", fileName, newFileName])
return newFileName
----------------
I'm not sure it's the responsibility of this check to do git stuff, there may
be use cases where this is not wanted / users might not expect this behavior.
Introducing a dependency to git might also make this script harder to unit test
in the future.
Thus I think it'd be better to keep the original behavior so the script has one
single responsibility. What do you think @njames93 ?
================
Comment at: clang-tools-extra/clang-tidy/rename_check.py:97
def getListOfFiles(clang_tidy_path):
- files = glob.glob(os.path.join(clang_tidy_path, '*'))
- for dirname in files:
- if os.path.isdir(dirname):
- files += glob.glob(os.path.join(dirname, '*'))
+ files = glob.glob(os.path.join(clang_tidy_path, '**'), recursive=True)
files += glob.glob(os.path.join(clang_tidy_path, '..', 'test',
----------------
Why is this change needed? I'd expect only line 99 to be needed to fix moving
the test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141463/new/
https://reviews.llvm.org/D141463
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
