Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename root -> cats in build.sbt #4644

Merged
merged 1 commit into from
Aug 26, 2024
Merged

Conversation

danicheg
Copy link
Member

To enhance the navigation in $IDE between different projects (right now, the 'cats' project displays as 'root' in the projects' list which is not informative).

@@ -372,7 +372,7 @@ jobs:
- name: Submit Dependencies
uses: scalacenter/sbt-dependency-submission@v2
with:
modules-ignore: rootjs_2.12 rootjs_2.13 rootjs_3 docs_2.12 docs_2.13 docs_3 cats-tests_sjs1_2.12 cats-tests_sjs1_2.13 cats-tests_sjs1_3 rootjvm_2.12 rootjvm_2.13 rootjvm_3 rootnative_2.12 rootnative_2.13 rootnative_3 cats-tests_2.12 cats-tests_2.13 cats-tests_3 bincompattest_2.12 bincompattest_2.13 bincompattest_3 cats-tests_native0.5_2.12 cats-tests_native0.5_2.13 cats-tests_native0.5_3 cats-bench_2.12 cats-bench_2.13 cats-bench_3
modules-ignore: cats-root_2.12 cats-root_2.13 cats-root_3 docs_2.12 docs_2.13 docs_3 cats-tests_sjs1_2.12 cats-tests_sjs1_2.13 cats-tests_sjs1_3 cats-root_2.12 cats-root_2.13 cats-root_3 cats-root_2.12 cats-root_2.13 cats-root_3 cats-tests_2.12 cats-tests_2.13 cats-tests_3 bincompattest_2.12 bincompattest_2.13 bincompattest_3 cats-tests_native0.5_2.12 cats-tests_native0.5_2.13 cats-tests_native0.5_3 cats-bench_2.12 cats-bench_2.13 cats-bench_3
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears that if the root submodule has a name set through the corresponding SBT settings, we get some duplications when generating modules-ignore (checked in other projects — it's the same). Interestingly, platform-based postfixes have also disappeared. Not quite sure if we should worry about this though. Tagging a notoriously known expert in the field, cc @armanbilge.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is the right way to make this change. What happens if you rename root to cats in lazy val root = tlCrossRootProject instead?

Copy link
Member Author

@danicheg danicheg Aug 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is the right way to make this change.

It's pretty common in dozens of projects I've seen so far (specifically, adjusting the name SBT setting for root-like submodules). IIUC, we can freely give up on the moduleName for the root submodule. However, this approach would differ slightly from what we do for the other submodules in the build. That said, it doesn't seem crucial to me.

Aside from that, I wonder why adjusting the moduleName for the root submodule overrides the corresponding names of the platform-based submodules. It should act as a prefix, like cats-rootJVM, cats-rootJS, etc.

@danicheg danicheg changed the title Set name & moduleName for the root module Rename root -> cats in build.sbt Aug 24, 2024
@danicheg
Copy link
Member Author

So, I applied Arman's suggestion to workaround the issue I noticed above #4644 (comment). Ultimately, it worked well for my original goal, and that issue passed away.

@danicheg danicheg requested review from armanbilge and satorg August 25, 2024 05:33
Copy link
Contributor

@satorg satorg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Comment on lines -403 to +404
scalafix:
name: Scalafix
site:
name: Generate Site
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm weirded out by all these other changes but CI is passing so 🤷

@danicheg
Copy link
Member Author

Yeah, that change in the CI workflow definition also confused me. But since it comes from the plugin we barely have anything to do with it 🤷

@danicheg danicheg merged commit 4e976f9 into typelevel:main Aug 26, 2024
16 checks passed
@danicheg danicheg deleted the root-module-name branch August 26, 2024 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants