-
Notifications
You must be signed in to change notification settings - Fork 17
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
Modulable avro plugin #196
Conversation
f2ebb3d
to
b76111f
Compare
e88e18e
to
be074f4
Compare
bridge/src/main/java/com/github/sbt/avro/AvroCompilerBridge.java
Outdated
Show resolved
Hide resolved
bridge/src/main/java/com/github/sbt/avro/AvroCompilerBridge.java
Outdated
Show resolved
Hide resolved
if (AvroVersion.getMinor() > 8) { | ||
compiler.setGettersReturnOptional(optionalGetters); | ||
} | ||
if (AvroVersion.getMinor() > 9) { | ||
compiler.setOptionalGettersForNullableFieldsOnly(optionalGetters); | ||
} |
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.
Shouldn't the AvroVersion.getMinor ifs also check major version?
Also I suppose maintaining this bridge class means we need to track the avro API changes with each new version to correctly pass the new parameters. Is that right?
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.
yes. If there is a new compiler parameter, we can add it to the sbt plugin but we must make sure that if the runtime avro version does not support it, we won't call the method.
7becea6
to
23e69a2
Compare
23e69a2
to
51f6065
Compare
Currently
sbt-avro
can only use a single avro compiler version in the whole build.This PR intend to change this by using a custom
avro
configuration, containing the required artifacts for the compilation phase.Other notable change: As the plugin now adds the
avro
dependency, it is not automatically triggered.