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

Make zinc-scripted show up in IntelliJ #1298

Closed
wants to merge 1 commit into from

Conversation

Friendseeker
Copy link
Member

@Friendseeker Friendseeker commented Dec 2, 2023

Fixes other half of #1296

Validate the fix:

Screenshot 2023-12-02 at 12 20 52 PM

(As shown in screenshot, smart IDE features are now working)

@Friendseeker Friendseeker marked this pull request as ready for review December 2, 2023 20:21
@Friendseeker
Copy link
Member Author

Friendseeker commented Dec 2, 2023

@SethTisue

Since this PR is changing some logics written by you, would you mind to take a look at this PR?

(and preferably also take a look at #1297, since both PR are closely related).


I will be busy for the rest of December since I have a couple exams to deal with, so I might not be able to immediately respond to PR feedbacks, but I will still try my best to address feedbacks ASAP.

@Friendseeker Friendseeker requested review from eed3si9n and removed request for SethTisue January 5, 2024 21:41
@@ -667,9 +666,6 @@ lazy val zincScripted = (projectMatrix in internalPath / "zinc-scripted")
)
.defaultAxes(VirtualAxis.jvm, VirtualAxis.scalaPartialVersion(scala212))
.jvmPlatform(scalaVersions = List(scala212))
.configure(
_.dependsOn(compilerBridge210, compilerBridge211, compilerBridge212, compilerBridge213)
Copy link
Member

Choose a reason for hiding this comment

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

This makes zincScripted depend on all compiler bridge JARs whereas the one above will only depend on the ones for Scala 2.12? I wonder if this was meant to create build order dependency so calling the scripted test would force rebuilding compiler bridges when we change the bridge imple. Could you check if that still works plz?

@SethTisue
Copy link
Member

SethTisue commented Jan 7, 2024

I no longer remember what I might have been thinking. I'm not even confident what commits you might be referring to — sbt/zinc@4d39439 ? regardless, probably I was just fumbling around

if you ask a more specific question it might ring a bell in my memory, but otherwise we should probably just rely on Eugene's say-so; I see he already merged the other PR

@eed3si9n
Copy link
Member

eed3si9n commented Apr 7, 2024

Closing this for now for inactivity. Feel free to reopen this if you want to pick this up.

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