Skip to content

Compiler classpath and DiagnosticCollector#2

Open
rlsutton1 wants to merge 1 commit into
verhas:masterfrom
rlsutton1:master
Open

Compiler classpath and DiagnosticCollector#2
rlsutton1 wants to merge 1 commit into
verhas:masterfrom
rlsutton1:master

Conversation

@rlsutton1

Copy link
Copy Markdown

adds ability to set and use a DiagnosticCollector
adds ability to set options, such as a class path
changed source version to 1.7

add ability to set options, such as a class path
changed source version to 1.7
public void setClassLoader(ClassLoader classLoader) {
this.classLoader = classLoader;
}
private ClassLoader classLoader = Compiler.class.getClassLoader();

@verhas verhas Jun 28, 2016

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General recommendation: when you make a change in a project do not reformat the code, keep the tabbing. Otherwise the 'diff' will show a lot of lines deleted and reinserted and it is hard to assess the true nature of the modification.

@verhas

verhas commented Jun 28, 2016

Copy link
Copy Markdown
Owner

Please remove the formatting change and follow the suggestions I gave about style (mainly have if/than/else structures explicitely rather than 'set default here instead of else branch / if/then return and omit else / return error'.

If you do that I will be more than happy to merge your changes into the project. These changes seem to be very good.

And a last comment: If there is any change in the code and there is no change in the test it means the quality of the code degraded. Create new test cases that test your changes.

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.

2 participants