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

feat: fallback to source and javadoc classifiers when exporting projects from sbt #2118

Merged
merged 2 commits into from Jul 18, 2023

Conversation

kpodsiad
Copy link
Collaborator

As a power user I often use bloopInstall from already opened sbt session as it is much faster than Metals: Import Build (warm JVM with sbt, etc.). However, there is one main advantage that Metals: Import Build has - it runs sbt with -Dbloop.export-jar-classifiers=sources.

Even though it is possible to configure bloop's sbt plugin to download sources, by tweaking build.sbt I don't consider it as a viable option because:

  • not everybody uses metals, hence bloop which is default build server
  • not everybody uses bloop as their default build server

Not having sources for dependencies results in really bad UX in Metals, as they can't e.g. go to the definition of given class. They instead fail silently, or go to the completely different class which happens to have the same symbol name.

I do understand that this may lead to slightly longer downloading times, but I think it's better to sacrifice a bit of time once, rather than UX every time when there are no sources.

@kpodsiad kpodsiad requested a review from tgodzik July 17, 2023 06:36
@tgodzik tgodzik force-pushed the kpodsiadlo/export-sources-from-sbt branch from a48818e to 8fe341f Compare July 17, 2023 11:25
@tgodzik
Copy link
Contributor

tgodzik commented Jul 17, 2023

Hmm... I tried rebase but broke something, let fix that in sec.

@tgodzik tgodzik force-pushed the kpodsiadlo/export-sources-from-sbt branch from 8fe341f to ea971c7 Compare July 17, 2023 11:27
@tgodzik tgodzik force-pushed the kpodsiadlo/export-sources-from-sbt branch from ea971c7 to 1a83daa Compare July 17, 2023 11:33
@kpodsiad
Copy link
Collaborator Author

Error:  x sbt-bloop / cross-compile-test-configurations 
Error:   Cause of test exception: {line 10}  Command failed: checkSourceAndDocs failed

Seems like my change broke something in classifiers? I'll have a look at it tomorrow.

@kpodsiad kpodsiad changed the title feat: fallback to source when exporting projects from sbt feat: fallback to source and javadoc classifiers when exporting projects from sbt Jul 18, 2023
@kpodsiad
Copy link
Collaborator Author

@tgodzik it seems that source and javadoc together made CI happy.

@tgodzik tgodzik merged commit f2dc891 into main Jul 18, 2023
17 checks passed
@tgodzik tgodzik deleted the kpodsiadlo/export-sources-from-sbt branch July 18, 2023 07:44
@@ -144,6 +144,7 @@ object BloopDefaults {
Option(System.getProperty("bloop.export-jar-classifiers"))
.orElse(Option(System.getenv("BLOOP_EXPORT_JAR_CLASSIFIERS")))
.map(_.split(",").toSet)
.orElse(Some(Set("sources", "javadoc")))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@tgodzik sources classifier is useful for metals, but is javadoc useful? I do wonder if this isn't too much, wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be useful in some potential cases, though it's unused currently in Metal

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.

None yet

2 participants