AVRO-3285: Upgrade JavaCC plugin and library #1447
Conversation
Also resolve IDE warnings for idl.jj, which was previously impossible.
| <groupId>org.apache.avro</groupId> | ||
| <version>1.12.0-SNAPSHOT</version> | ||
| <relativePath>../</relativePath> | ||
| <relativePath>../pom.xml</relativePath> |
There was a problem hiding this comment.
Minor really, but the relative path should point to a pom.
| <argument>${project.basedir}/src/test/resources/full_record_v1.avsc</argument> | ||
| <argument>${project.basedir}/src/test/resources/full_record_v2.avsc</argument> | ||
| <argument>${project.basedir}/target/generated-test-sources</argument> | ||
| <argument>${project.basedir}/target/generated-test-sources/javacc</argument> |
There was a problem hiding this comment.
Generated code should be placed in a subdirectory per tool. I copied the directory name from line 178.
| <configuration> | ||
| <sources> | ||
| <source>${project.basedir}/target/generated-test-sources</source> | ||
| <source>${project.basedir}/target/generated-test-sources/javacc</source> |
There was a problem hiding this comment.
Mirror the change in line 140 to maintain a working build.
| import com.fasterxml.jackson.databind.node.*; | ||
|
|
||
| import org.apache.commons.lang3.StringEscapeUtils; | ||
| import org.apache.commons.text.StringEscapeUtils; |
There was a problem hiding this comment.
org.apache.commons.lang3.StringEscapeUtils is deprecated in favor of org.apache.commons.text.StringEscapeUtils.
| this.resourceLoader = parent.resourceLoader; | ||
| } | ||
|
|
||
| @SuppressWarnings("RedundantThrows") |
There was a problem hiding this comment.
A bit silly, but this is the one thing I cannot fix.
| <groupId>org.javacc.plugin</groupId> | ||
| <artifactId>javacc-maven-plugin</artifactId> | ||
| <version>${javacc-plugin.version}</version> | ||
| <dependencies> |
There was a problem hiding this comment.
The new plugin treats JavaCC as a provided dependency, so we need to add it explicitly.
| | "timestamp_ms" { return LogicalTypes.timestampMillis().addToSchema(Schema.create(Type.LONG)); } | ||
| | "local_timestamp_ms" { return LogicalTypes.localTimestampMillis().addToSchema(Schema.create(Type.LONG)); } | ||
| | "decimal" { return DecimalTypeProperties(); } | ||
| | "decimal" s = DecimalTypeProperties() { return s; } |
There was a problem hiding this comment.
what is the benefit of this change ?
There was a problem hiding this comment.
Nothing directly, but the rule DecimalTypeProperties no longer shows up as 'unused in IntelliJ.
|
Thank you for the contribution, @opwvhk ! |
|
I think this should not be backported to branch-1.11. |
|
Correct @martin-g: it's an improvement, not a fix for a bug, so there's no need. |
Also resolve IDE warnings for idl.jj, which was previously impossible.
3 participants
Also resolve IDE warnings for idl.jj, which was previously impossible.
Make sure you have checked all steps below.
Jira
Tests
adds the following unit tests ORdoes not need testing for this extremely good reason:Upgrading plugins and libraries does not add functionality; existing tests must pass though
Commits
Documentation