Kill mill.modules package #2513
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lihaoyi
force-pushed
the
kill-mill-modules
branch
from
f8230c1
to
43f86cf
Compare
lefou
reviewed
May 10, 2023
There was a problem hiding this comment.
Looks good to me, but we need some @deprecated type aliases and forwarder objects to keep source compatibility for a while. Migration and cross compilation is otherwise a pita and especially, when not the whole ecosystem is migrated, it's less attractive to try something out if there is no easy going back.
|
Added backwards compat forwarders. Needed to put them in |
lefou
reviewed
May 11, 2023
lefou
approved these changes
May 11, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ticks off #1826 (comment)
mill.modulesis a very confusing name that I came up with very early in Mill's development. It misleadingly has nothing to do withmill.Module, and is just a grab bag of helpers. This PR moves them to the JVM packages which are most related to their functionalityWe basically move the following files:
mill.modules.Assembly->mill.scalalib.Assemblymill.modules.Util->mill.util.Utilmill.modules.Jvm->mill.util.Jvmmill.modules.CoursierSupport->mill.util.CoursierSupportThere is probably further cleanup and re-organization we can do here. Currently,
UtilandJvmare pretty tightly coupled together, withVisualizeModuleliving inmainstopping us from moving them all toscalalib. An alternative is to moveAssemblytomill.util, so at leastcreateJarandcreateAssemblycan live in the same package rather than being split acrossmill.{util,scalalib}.But for now, just breaking up
mill.modulesshould already be a step forward, and further cleanups can come in follow up PRs