[PATCH] D141463: [clang-tidy] Improve rename_check.py

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 12 12:35:57 PST 2023


njames93 added inline comments.


================
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
----------------
ccotter wrote:
> carlosgalvezp wrote:
> > 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 ?
> I wasn't sure about this - reverted this. Single responsibility is better.
Yeah we shouldn't be messing with git indexes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141463



More information about the cfe-commits mailing list