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

Refined fix for ScoverageModule inner module #2584

Merged
merged 3 commits into from
Jun 8, 2023
Merged

Refined fix for ScoverageModule inner module #2584

merged 3 commits into from
Jun 8, 2023

Conversation

lefou
Copy link
Member

@lefou lefou commented Jun 8, 2023

This is a follow-up on #2583.

Using a ModuleRef make accesss to the inner module for the CLI impossible.
I added that case to the integration test and changed to use a val.

lefou added 2 commits June 8, 2023 14:35
Using a ModuleRef make accesss to the inner module for the CLI impossible.
I added that case to the integration test and changed to use a `val`.
@lefou lefou changed the title Refined fix for Scoverage inner module Refined fix for ScoverageModule inner module Jun 8, 2023
@lihaoyi
Copy link
Member

lihaoyi commented Jun 8, 2023

Yeah ModuleRef was added somewhat intentionally as a workaround to allow you to refer to a modulenwithout making it visible from the CLI; e.g. you don't want everyone's zincWorker to be spamming your mill resolve __. Going back to def for modules you do want to be visible sounds reasonable

@lefou lefou merged commit 1671839 into main Jun 8, 2023
@lefou lefou deleted the scov-fix branch June 8, 2023 14:42
@lefou lefou added this to the 0.11.1 milestone Jun 8, 2023
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.

[error] class type required but {some traits} found _root_.mill.define.Discover[build]
2 participants