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

Add InteliiJ aspect info #242

Merged
merged 4 commits into from
Jan 15, 2020
Merged

Add InteliiJ aspect info #242

merged 4 commits into from
Jan 15, 2020

Conversation

szinn
Copy link
Contributor

@szinn szinn commented Dec 14, 2019

- 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
@cgruber
Copy link
Collaborator

cgruber commented Dec 16, 2019

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?

@szinn
Copy link
Contributor Author

szinn commented Dec 17, 2019

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)

@szinn
Copy link
Contributor Author

szinn commented Jan 14, 2020

@cgruber This change has been updated to put the language_version in the KtJvmInfo struct.

@cgruber
Copy link
Collaborator

cgruber commented Jan 15, 2020

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.

@cgruber cgruber merged commit a3d4d56 into bazelbuild:master Jan 15, 2020
@szinn
Copy link
Contributor Author

szinn commented Jan 15, 2020 via email

@cgruber
Copy link
Collaborator

cgruber commented Jan 15, 2020

I'll be releasing 1.3.1 this weekend or next week, I expect, so it'll be available very soon.

cgruber added a commit to cgruber/rules_kotlin that referenced this pull request Feb 10, 2020
* 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)
cgruber added a commit to cgruber/rules_kotlin that referenced this pull request Feb 10, 2020
* 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.
jongerrish added a commit to jongerrish/rules_kotlin that referenced this pull request Apr 16, 2020
- 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants