-
Notifications
You must be signed in to change notification settings - Fork 161
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
fix: we now only add -g
flag to Java source types
#1657
Conversation
@@ -25,7 +27,9 @@ protected JavaSource(ResourceRef ref, String script, Function<String, String> re | |||
|
|||
@Override | |||
protected List<String> getCompileOptions() { | |||
return tagReader.collectOptions("JAVAC_OPTIONS", "COMPILE_OPTIONS"); | |||
List<String> gopts = Collections.singletonList("-g"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not really a fan of adding this here - won't this mean that if you have 10 source files you get 10 x "-g" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly, yes. For now I just don't know where else to put it :-/
For some time now I've thought that there should probably be a class for each kind of project, eg JavaProject, KotlinProject, etc. Then we'd have a proper place to put things like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can of course leave it where it is and surround it with an instanceof test to see if the main source file is JavaSource or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe have a LinkedSet contributeGlobalCompileOptions();
which allows a Source to add to a set that gets added first?
Before it would get added to Groovy and Kotlin as well. Fixes jbangdev#1656
I tried coming up with a nice solution, by introducing different kinds of Projects but it got a little out of hand. So I need to rethink that at some point. So for now I'm using an ugly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not great but better than annoying fail.
Before it would get added to Groovy and Kotlin as well.
Fixes #1656