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

Move task override tracking from planning time to instantiation time #4181

Merged
merged 26 commits into from
Feb 4, 2025

Conversation

lihaoyi
Copy link
Member

@lihaoyi lihaoyi commented Dec 26, 2024

Fixes #4176

  • Make overriding of Discover mandatory, even in tests, so it always has useful data
  • Propagate implicitMillDiscover throughout the module hierarchy implicitly through Ctx
  • Combine Discover#classInfo#declaredTasks#name together with the linearization of the Scala trait hierarchy for each module, to decide whether or not a specific task is overridden or not
  • Added a new unit test optionalOverride that fails on main and passes on this PR

Also took the opportunity to break up TestGraphs, EvaluationTests and ResolveTests a bit. Still pretty messy, but better than before

@lihaoyi lihaoyi marked this pull request as ready for review February 4, 2025 05:36
@@ -17,6 +18,8 @@ object BuildInfoTests extends TestSuite {
def scalaVersion = scalaVersionString
def buildInfoPackageName = "foo"
def buildInfoMembers = Seq.empty[BuildInfo.Value]

def millDiscover = Discover[this.type]
Copy link
Member

@lefou lefou Feb 4, 2025

Choose a reason for hiding this comment

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

Shouldn't this be a val or lazy val, here and in all the other places? The object is already at the end of the inheritance hierarchy, and the def may cause wasteful re-initialisation of Discover.

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably can be a lazy val

@lihaoyi lihaoyi merged commit 23563b4 into com-lihaoyi:main Feb 4, 2025
29 of 31 checks passed
@lefou lefou added this to the 0.13.0 milestone Feb 4, 2025
lihaoyi added a commit that referenced this pull request Feb 5, 2025
Since #4181 landed, we no longer
need to track `Terminal.Labelled#segments` separately and can just use
`Terminal.Labelled#task#ctx#segments#render`
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.

Move override segment suffixing from planning time to task instantiation time
2 participants