[rebased/cherry-picked] Switch to JarsToLabels provider and rework/cleanup scala_import #487
Conversation
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
andyscott
changed the title
|
|
|
Is this really a good idea to pile up a few different changes on a big
refactoring?
It makes reviewing much harder.
I’m still not sure about the core change and exports...
…On Sun, 29 Apr 2018 at 21:01 andy g scott ☜ ***@***.***> wrote:
I goofed-- this will need some more work to make sure all the changes were
pulled over correctly.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#487 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIFwYP9oXDRJ5r6RoULMArFY6dOtJCks5ttf_0gaJpZM4Tr4rY>
.
|
|
I'm not too attached to this work, but I'll have to defer to @jjudd since he added the more recent changes to make the original refactoring support IntelliJ. Also, given some recent discussions, I wanted to make sure we had a more manageable branch based on recent refactoring. |
|
We are running on a branch that has all these changes on it, and things have been working fine. I'm not too attached to this work either, as we'll be switching to Annex internally this week for the Zinc support. |
|
Also, I consent to my changes being contributed. |
|
:)
I’ll try to review it as is and hopefully we’ll be able to push it in
…On Sun, 29 Apr 2018 at 22:14 James Judd ***@***.***> wrote:
We are running on a branch that has all these changes on it, and things
have been working fine. I'm not too attached to this work either, as we'll
be switching to Annex internally this week for the Zinc support.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#487 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIFyrlPagAU5byoLNJLhjgghbdoLoNks5tthEGgaJpZM4Tr4rY>
.
|
|
There's a test or two that's currently broken. I'll have to poke at it later. |
| # support http_file pointed at a jar. http_jar uses ijar, | ||
| # which breaks scala macros | ||
| # support http_file pointed at a jar. http_jar uses ijar, | ||
| # which breaks scala macros |
There was a problem hiding this comment.
this comment is on the elif, can we move it below or unindent? It feels awkward to me being indented but not related to the current block.
| @@ -30,3 +30,10 @@ def create_scala_provider( | |||
| transitive_runtime_jars = transitive_runtime_jars, | |||
| transitive_exports = [] #needed by intellij plugin | |||
| ) | |||
|
|
|||
| JarsToLabels = provider( | |||
There was a problem hiding this comment.
can we make a link to the bazel discussions about putting labels in the jars?
|
Any progress on this? Would be unfortunate for this to bitrot when we're so close to getting this in. |
|
I agree but also introducing bugs in a critical flow would be unfortunate.
Andi,
Did you get a chance to look at the exports thing?
…On Sat, 5 May 2018 at 11:24 David Flemström ***@***.***> wrote:
Any progress on this? Would be unfortunate for this to bitrot when we're
so close to getting this in.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#487 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIF3-benm5cAfDK0r8wTSO6pQUoqzwks5tvWG5gaJpZM4Tr4rY>
.
|
This is a rebased version of #477 plus the addition of @jjudd's changes in https://github.com/andyscott/rules_scala/pull/2.