Hi,
all clang libs are usually in clang/lib/Foo/Bar and are the called clangFooBar.
However, lib/Tooling/Refactoring is called clangToolingRefactor without the “ing”.
Can we make that consistent? Would you prefer if we renamed the directory or the library?
(I can do the actual renaming if we agree that it’s a good thing to do.)
Nico
Agreed that it’s a bit confusing. I have no preference on what the final (consistent) name should be.
While I personally like consistent naming myself, I’d prefer to be conservative with this one and avoid changing something that is not broken for other reasons and was like this for years.
While I personally like consistent naming myself, I’d prefer to be conservative with this one and avoid changing something that is not broken for other reasons and was like this for years.
I find this kind of argument not very convincing. See e.g. the first 7 slides of https://www.usenix.org/sites/default/files/conference/protected-files/srecon17americas_slides_reese.pdf
We’ve renamed many libraries to increase consistency, and we know from experience it’s a pretty safe thing to do.
If we do rename, do folks prefer:
Renaming the directory to lib/clang/Tooling/Refactor. Requires updating all #include lines referring to it, and updating a handful of CMake files.
Renaming the library to clangToolingRefactoring. Requires updating all cmake files adding a dependency to use the new library name.
I don’t think it’s risky, I think it does not add much value, but would require some work and would be a little inconvenient for those touching this code (merge conflicts, integrates, etc).
If an option not to do it is out of the table, I’d vote to rename the CMake target as that seems to cause strictly less problems: no merge conflicts due to moved files, no changes needed to source code outside build files.
While I personally like consistent naming myself, I’d prefer to be conservative with this one and avoid changing something that is not broken for other reasons and was like this for years.
I find this kind of argument not very convincing. See e.g. the first 7 slides of https://www.usenix.org/sites/default/files/conference/protected-files/srecon17americas_slides_reese.pdf
We’ve renamed many libraries to increase consistency, and we know from experience it’s a pretty safe thing to do.
I’ve dealt with the fallout from one of these renames recently - we silently lost some changes during a (non-git, non-svn) merge.
The unsafeness of it may not be visible from upstream LLVM 
If we do rename, do folks prefer:
Renaming the directory to lib/clang/Tooling/Refactor. Requires updating all #include lines referring to it, and updating a handful of CMake files.
Renaming the library to clangToolingRefactoring. Requires updating all cmake files adding a dependency to use the new library name.
Renaming the library is a less invasive change, less likely to screw with out-of-tree modifications, pending patches, other build systems.
So unless anyone has a strong opinion on what the better name is (I don’t), I’d prefer #2.
While I personally like consistent naming myself, I’d prefer to be conservative with this one and avoid changing something that is not broken for other reasons and was like this for years.
I find this kind of argument not very convincing. See e.g. the first 7 slides of https://www.usenix.org/sites/default/files/conference/protected-files/srecon17americas_slides_reese.pdf
We’ve renamed many libraries to increase consistency, and we know from experience it’s a pretty safe thing to do.
I’ve dealt with the fallout from one of these renames recently - we silently lost some changes during a (non-git, non-svn) merge.
The unsafeness of it may not be visible from upstream LLVM
I agree. That said, the fact that we accept upstream churn without regard to out-of-tree users (even when those users are ourselves!) is a forcing function that we use to encourage people to upstream their changes. It’s unsustainable to hold back upstream cleanups in order to make it easier to maintain out-of-tree patches.
If we do rename, do folks prefer:
Renaming the directory to lib/clang/Tooling/Refactor. Requires updating all #include lines referring to it, and updating a handful of CMake files.
Renaming the library to clangToolingRefactoring. Requires updating all cmake files adding a dependency to use the new library name.
Renaming the library is a less invasive change, less likely to screw with out-of-tree modifications, pending patches, other build systems.
So unless anyone has a strong opinion on what the better name is (I don’t), I’d prefer #2.
+1 to fixing the build system to give the library a consistent name.
I’m fine with renaming the directory lib/Tooling/Refactoring to lib/Tooling/Refactor. Renaming the library to be consistent is a good step forward as well, so thumbs up from me. If there’s still interest after renaming the library, I can make a patch that renames the directory as well. We have some not-yet-upstreamed code in apple/swift-clang on Github that will be affected, but I will take care of it.
Thanks,
Alex