-
Notifications
You must be signed in to change notification settings - Fork 275
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
Add support for an explicit srcjar for scala_import #486
Comments
Hi David,
Thanks for reaching out!
I think we’re in the midst of a few changes (see move to new providers,
refactoring of the whole files and in general discussion of new JavaInfo).
I think that in a few weeks the dust will settle and we’ll tackle this. How
does that sound?
…On Fri, 27 Apr 2018 at 16:52 David Flemström ***@***.***> wrote:
It would be nice to have scala_import be on par with java_import
<https://docs.bazel.build/versions/master/be/java.html#java_import> in
this regard, especially since scala_import_external suggests it should be
supported.
I could probably take on fixing this if somebody could mentor me regarding
a preferred solution.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#486>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIF4SMdFe8GcgkXrUGccpyh5t3bQ7Xks5tsyK6gaJpZM4TqaBI>
.
|
Sounds good, I mostly wanted to create this issue to have something to track progress, it's not very urgent at all. If I happen to find the time to fix it on top of the old |
We added We currently use it with I'm not totally certain the status of that PR. We're busy rolling out Bazel for Scala to all of engineering internally over the next two weeks and have a bunch of forks that we'll need to clean up and get back onto master once we've migrated. |
Right, that's part of the turmoil I was referring too since I remembered
discussion about it but didn't remember it taking place on a PR.
That PR is currently stuck on intellij not actually using the srcjar so the
plan IIRC is to fix intellij plugin and then move forward with it
…On Fri, Apr 27, 2018 at 5:39 PM James Judd ***@***.***> wrote:
We added srcjar support for Scala import in this commit:
***@***.***
<andyscott@a457c06>,
which is currently PR to this PR: #477
<#477>
We currently use it with scala_import_external.
I'm not totally certain the status of that PR. We're busy rolling out
Bazel for Scala to all of engineering internally over the next two weeks
and have a bunch of forks that we'll need to clean up and get back onto
master once we've migrated.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#486 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIF7t-YeRKfcBuSkuTG7zvbXwCq8wsks5tsy2mgaJpZM4TqaBI>
.
|
If my PR gets merged to that PR, then it supports IntelliJ as well as rules_scala currently does. If we want, we could move forward with the PR now and then add the support to IntelliJ separately from that PR. |
This dependency knot is a pain. I am losing state. I think bazel has to be updated for intellij to not lose state using only JavaInfo. So, that has to land, then we have to send the PR to the plugin, then we have to update locally. @jjudd says we don't see a regression if we merge now. Can anyone confirm that? @andyscott @gkk-stripe ? If we merge and regress on intellij it will be an issue for us at Stripe, and probably others. |
@johnynek agreed. It's spaghetti right now. We'll need this andyscott@a457c06 merged into @andyscott's branch before we merge if we want IntelliJ to continue working. |
I'll take a look at the PR to my side branch tomorrow (Sunday).
…On Fri, Apr 27, 2018, 11:07 James Judd ***@***.***> wrote:
@johnynek <https://github.com/johnynek> agreed. It's spaghetti right now.
@johnynek <https://github.com/johnynek> We'll need this
***@***.***
<andyscott@a457c06>
merged into @andyscott <https://github.com/andyscott>'s branch before we
merge if we want IntelliJ to continue working.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#486 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAS8W-PrYroeevDRvMRg2LawahpkLpoyks5ts15ugaJpZM4TqaBI>
.
|
See #487 for the sum of all lines of development. |
It would be nice to have
scala_import
be on par withjava_import
in this regard, especially sincescala_import_external
suggests it should be supported.I could probably take on fixing this if somebody could mentor me regarding a preferred solution.
The text was updated successfully, but these errors were encountered: