Skip to content

Conversation

@mrsarm
Copy link

@mrsarm mrsarm commented Mar 2, 2020

mrsarm added 3 commits March 2, 2020 17:04
…path logging

- Add dependsOn 'classes' only if the task is registered, otherwise let the task without dependencies and warn that the project need to be compiled first (multi-modules projects may not have the :classes task available)
- Takes classpath paths from project and subprojects if exist. Also use `JavaExec` instead of `JavaCompile` to scan not just the "implementation" dependencies (or "compile") but also scan the "runtimeOnly" dependencies (or "runtime")
- Add some logging
- Remove unused imports
…e internal class `JShellToolProvider`

- The change allows to avoid the need to use special java options to run the task
- Improvements in README doc
@mrsarm
Copy link
Author

mrsarm commented Mar 4, 2020

Hi @bitterfox , did you have the chance to check my PR? Is there something you think need to be fixed first?

@vbrandl
Copy link

vbrandl commented Mar 18, 2020

I would love to see this merged. #6 is currently blocking me from using this plugin.

@mrsarm
Copy link
Author

mrsarm commented Jun 25, 2020

@vbrandl I made a fork a few weeks ago with the patch and other improvements, and is also published in the Gradle plugins center: https://github.com/mrsarm/jshell-plugin

@vbrandl
Copy link

vbrandl commented Jun 26, 2020

@mrsarm thanks for the heads up. I'll look into it at work on Monday.

@bitterfox
Copy link
Owner

Hi, @mrsarm
Thank you for your contribution and I really apologize that I wasn't aware of your contribution for longer.
I saw your fork on Twitter recently and I noted your pull request.
I really love to see your respectful fork! You improved my PoC project from various aspects, implementations, features, documentation, and a lot! 👍

I'd like to mention/introduce your fork in README, but may I?

Also please tell me how should I handle this PR. I'm welcome to merge this, but I think your code is already used in your fork.
So I respect your preference.

Again, thank you for your contribution and great forks and I really apologize for your inconvenience.

@mrsarm
Copy link
Author

mrsarm commented Jul 21, 2021

Hi @bitterfox , thanks for your comments !

I'd like to mention/introduce your fork in README, but may I?

Sure, please do.

Also please tell me how should I handle this PR. I'm welcome to merge this, but I think your code is already used in your fork.
So I respect your preference.

Yes, if you plan to not maintain this repo anymore there is no need to merge it. Furthermore, in the v1.1.0 of my fork I added changes not reflected here. The most important change is how the task is registered in Gradle, the old way made the plugin not working well with Java 12+.

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.

3 participants