-
Notifications
You must be signed in to change notification settings - Fork 212
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 InteliiJ aspect info #242
Conversation
- allows the bazel-intellij aspect processing to correctly identify the version of Kotlin being used and update the source accordingly - part of the fix for bazelbuild/intellij#1274
- allows the bazel-intellij aspect processing to correctly identify the version of Kotlin being used and update the source accordingly - part of the fix for bazelbuild/intellij#1274
So this seems reasonable to me, but I'm curious about passing it as a separate part of the struct, rather than baking it in to values in KtJvmInfo. Is there a strong reason to prefer that? Is there prior art on passing toolchains in that providers struct? I see examples of passing back ToolchainInfo from rules that define the toolchain, but this feels different. I'm struggling to see a good example anywhere to resolve this in my mind. This isn't a toolchain rule you're editing. Also, do you need the stdlib compilation path? (or is that just to force the stdlibs into the IDE thingy?) Is this not something you can attach to an aspect? The PR description references aspects, but I don't see an aspect given here (and the intellij aspect lives elsewhere). Could you possibly get the ctx.rule.toolchain in the intellij aspect, to get this information? |
I was seeing that this kind of information was passed back in the toolchain as you described. As well, the version of the tool is more a toolchain info, but could easily be passed in the KtJvmInfo as well and extracted from there. The stdlib path was purely to fill out the intellij structure. Perhaps it isn't needed there and just the version can be put into the KtJvmInfo. Shall I make this change to the PR? As well, this change does nothing for the tool, it is putting the information into the return structure so that it can be extracted by the intellij aspect code (which is the other PR I shared) |
@cgruber This change has been updated to put the language_version in the KtJvmInfo struct. |
I'm just assuming this can be picked up on other side by the plugin (or its aspect). But this change is fine, as it is, assuming something will use it. |
I have a change for bazel-intellij to pick up this change
…On Wed, Jan 15, 2020 at 5:21 PM Christian Edward Gruber < ***@***.***> wrote:
I'm just assuming this can be picked up on other side by the plugin (or
its aspect). But this change is fine, as it is, assuming something will use
it.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#242?email_source=notifications&email_token=AADN2ICTC4BVSZLLVX5XIO3Q56D7BA5CNFSM4J24WD22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJCAVIA#issuecomment-574884512>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADN2IGK5FDPCCPSNA5FHELQ56D7BANCNFSM4J24WD2Q>
.
|
I'll be releasing 1.3.1 this weekend or next week, I expect, so it'll be available very soon. |
* upstream/master: typo and add missing paren. Add release legacy-1.3.0 Reference proper "current release" as legacy 1.3.0 Update README to account for legacy-1.3.0 release. Fix bad date in changelog Release the Kotlin rules in a simplified workspace relying on precompiled jars. (bazelbuild#271) Update CODEOWNERS (remove str, thomaswk) (bazelbuild#272) Factor out external workspace versions and SHA256s into variables conveniently placed at the top. (bazelbuild#259) Add a link to the generated documentation (bazelbuild#237) Stop using native java rules (bazelbuild#258) Add InteliiJ aspect info (bazelbuild#242) Change readme file formatting (bazelbuild#243) Update CHANGELOG.md Update readme to release candidate 4 Tweak the inference of the test class's package a bit, to support src/test/kotlin/ and kotlin/, since while it's common for kotlin files to live in a /java/ root, it's not UNcommon for them to live in a *kotlin/ root. (bazelbuild#257) Passthrough tags to the container android_library (bazelbuild#255) Fix current "latest" to actually be the latest. Update CHANGELOG.md Update to release legacy-1.3.0-rc3 Support more recent java versions in the kotlin toolchain. (bazelbuild#236)
* origin/master: Stop using native java rules (bazelbuild#258) Add InteliiJ aspect info (bazelbuild#242) Change readme file formatting (bazelbuild#243) Update CHANGELOG.md Update readme to release candidate 4 Tweak the inference of the test class's package a bit, to support src/test/kotlin/ and kotlin/, since while it's common for kotlin files to live in a /java/ root, it's not UNcommon for them to live in a *kotlin/ root. (bazelbuild#257) Passthrough tags to the container android_library (bazelbuild#255) Fix current "latest" to actually be the latest.
- supplies the language version in the kt provider - allows the bazel-intellij aspect processing to correctly identify the version of Kotlin being used and update the source accordingly - part of the fix for bazelbuild/intellij#1274
of Kotlin being used and update the source accordingly