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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits