8000 Add `Dep.millProjectModule` to refer to mill project modules by lefou · Pull Request #4790 · com-lihaoyi/mill · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add Dep.millProjectModule to refer to mill project modules #4790

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 16 commits into from
Mar 27, 2025
Merged

Add Dep.millProjectModule to refer to mill project modules #4790

merged 16 commits into from
Mar 27, 2025

Conversation

lefou
Copy link
Member
@lefou lefou commented Mar 27, 2025 •
edited
Loading

This is meant as a replacement for mill.util.ModuleUtils.millProjectModule.

The main motivation for this change is to handle mill module dependencies more regular. Since they can be resolved together with other dependencies, this should result in better conflict resolution. The older API rather forces us to concat two potential redundant or conflicting classpaths.

There is one last usage of the older ModuleUtils.millProjectModule from VisualizeModule, which resides in mill.main and can't access CoursierModule or Dep. Don't know how to best handle it.

@lefou lefou requested review from alexarchambault and lihaoyi and removed request for alexarchambault March 27, 2025 09:53
@lefou lefou marked this pull request as ready for review March 27, 2025 10:18
lihaoyi
lihaoyi approved these changes Mar 27, 2025
View reviewed changes
Copy link
Member
@lihaoyi lihaoyi left a comment

Choose a reason for hiding this comment

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

If the tests pass looks good to me I guess, this whole millModule thing is purely to support testing using local classes

@lihaoyi
Copy link
Member
lihaoyi commented Mar 27, 2025

There is one last usage of the older ModuleUtils.millProjectModule from VisualizeModule, which resides in mill.main and can't access CoursierModule or Dep. Don't know how to best handle it.

That's actually an interesting question to ask. Maybe we should have a "global" defaultResolver() to use when you are not part of any CoursierModule? That would be useful in cases such as PalantirFormatBaseModule where the companion object is a ExternalModules inherits from CoursierModule to get access to defaultResolver(), but really isn't a CoursierModule at all in the sense that the JavaModule/ScalaModules in your build config are.

@lefou lefou force-pushed the dep-mill-modules branch from 032eaca to 975e222 Compare March 27, 2025 14:36
@lefou
Copy link
Member Author
lefou commented Mar 27, 2025 •
edited
Loading

@lihaoyi That sound like a good idea. Since coursier is currently part of Mill core anyway, it should work. But it means we also need to move Dep and maybe some other classes up. Maybe into mill.api. I think I will leave the deprecated call for now, so we can discuss and refactor in a follow up.

@lefou
Copy link
Member Author
lefou commented Mar 27, 2025

The one test failing in CI is running successfully on my machine. Merging.

@lefou lefou merged commit 353f304 into com-lihaoyi:main Mar 27, 2025
61 of 63 checks passed
@lefou lefou deleted the dep-mill-modules branch March 27, 2025 22:18
@lefou lefou added this to the 0.13.0 milestone Mar 27, 2025
lefou added a commit to lefou/mill that referenced this pull request Mar 31, 2025
@lefou
Add Dep.millProjectModule to refer to mill project modules (backport …
9b91c18 
…com-lihaoyi#4790)

This is meant as a replacement for
`mill.util.ModuleUtils.millProjectModule`, which I annotated as deprecated.

The main motivation for this change is to handle mill module
dependencies more regular. Since they can be resolved together with
other dependencies, this should result in better conflict resolution.
The older API rather forces us to concat two potential redundant or
conflicting classpaths.

There is one last usage of the older `ModuleUtils.millProjectModule`
from `VisualizeModule`, which resides in `mill.main` and can't access
`CoursierModule` or `Dep`. Don't know how to best handle it.

Original pull request: com-lihaoyi#4790
@lefou lefou mentioned this pull request Mar 31, 2025
Merged
lefou added a commit to lefou/mill that referenced this pull request Mar 31, 2025
@lefou
Add Dep.millProjectModule to refer to mill project modules (backport …
656d44d 
…com-lihaoyi#4790)

This is meant as a replacement for
`mill.util.ModuleUtils.millProjectModule`, which I annotated as deprecated.

The main motivation for this change is to handle mill module
dependencies more regular. Since they can be resolved together with
other dependencies, this should result in better conflict resolution.
The older API rather forces us to concat two potential redundant or
conflicting classpaths.

There is one last usage of the older `ModuleUtils.millProjectModule`
from `VisualizeModule`, which resides in `mill.main` and can't access
`CoursierModule` or `Dep`. Don't know how to best handle it.

Original pull request: com-lihaoyi#4790
lefou added a commit that referenced this pull request Mar 31, 2025
@lefou
Add Dep.millProjectModule to refer to mill project modules (backport …
b3148f2 
…#4790) (#4830)

This is meant as a replacement for
`mill.util.ModuleUtils.millProjectModule`, which I annotated as
deprecated.

The main motivation for this change is to handle mill module
dependencies more regular. Since they can be resolved together with
other dependencies, this should result in better conflict resolution.
The older API rather forces us to concat two potential redundant or
conflicting classpaths.

There is one last usage of the older `ModuleUtils.millProjectModule`
from `VisualizeModule`, which resides in `mill.main` and can't access
`CoursierModule` or `Dep`. Don't know how to best handle it.

Original pull request: #4790

Pull request: #4830
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lihaoyi lihaoyi lihaoyi approved these changes

@alexarchambault alexarchambault alexarchambault approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.13.0-M1
Development

Successfully merging this pull request may close these issues.
3 participants
@lefou @lihaoyi @alexarchambault
0