-
Notifications
You must be signed in to change notification settings - Fork 119
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
Support alternate JavaCompilers (such as ECJ) in JavaSourcesSubject #80
base: main
Are you sure you want to change the base?
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
I signed it! |
CLAs look good, thanks! |
@@ -56,7 +57,8 @@ public Statement apply(final Statement base, Description description) { | |||
return new Statement() { | |||
@Override public void evaluate() throws Throwable { | |||
final AtomicReference<Throwable> thrown = new AtomicReference<Throwable>(); | |||
Result result = Compilation.compile(ImmutableList.of(new AbstractProcessor() { | |||
Result result = Compilation.compile(ToolProvider.getSystemJavaCompiler(), |
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.
Would be even better if you somehow merged #71 into that one wrt passing a JavaCompiler
to the CompilationRule
rather than always using ToolProvider.getSystemJavaCompiler()
. Sure one can easily be rebased on the other anyway…
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.
Sure, as soon as @tbroyer fixes the file manager thing
👍 Lacking tests running with the Eclipse compiler though to make sure everything works OK. |
Actually, I'd be happy to see this and #71 get merged into one p/r. |
Hello here, no new ? #71 is still not merged yet so I'm stuck with my PR ... |
New syntax to inject custom compiler: