Proposed changes to the update #42
Conversation
|
|
||
| <properties> | ||
| <dep.grpc.version>1.65.0</dep.grpc.version> | ||
| <dep.protobuf.version>3.25.4</dep.protobuf.version> | ||
| <dep.guava-bom.version>33.3.1-jre</dep.guava-bom.version> | ||
| <forkCount>1</forkCount> |
There was a problem hiding this comment.
We don't run the tests that used this, so I removed it (and the test run that didnt run tests).
| <dep.guava-bom.version>33.3.1-jre</dep.guava-bom.version> | ||
| <forkCount>1</forkCount> | ||
| <dep.guava.version>33.3.1-jre</dep.guava.version> | ||
| <protoOutDir>${project.build.directory}/generated-sources/protobuf</protoOutDir> |
There was a problem hiding this comment.
extracted this rather than list the string three times (previously twice, but i added a new usage)
| <groupId>com.google.api.grpc</groupId> | ||
| <artifactId>proto-google-common-protos</artifactId> | ||
| <version>1.12.0</version> | ||
| <scope>test</scope> |
There was a problem hiding this comment.
We never ran any tests with this, so I removed it.
| @@ -97,69 +74,6 @@ | |||
| </extension> | |||
| </extensions> | |||
| <plugins> | |||
| <plugin> | |||
| <groupId>org.apache.maven.plugins</groupId> | |||
| <artifactId>maven-shade-plugin</artifactId> | |||
There was a problem hiding this comment.
We produced a shaded build and also an assembly, but I'm not clear why - with a shaded copy of protobuf? Note too that this only applied to BrowserFlight, and not Barrage itself.
| <plugin> | ||
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-dependency-plugin</artifactId> | ||
| <groupId>com.mycila</groupId> |
There was a problem hiding this comment.
previous artifacts had no headers in the proto output - not sure it matters, but now we have them.
| </plugin> | ||
| <plugin> | ||
| <artifactId>maven-enforcer-plugin</artifactId> | ||
| <version>3.1.0</version> | ||
| </plugin> | ||
| <plugin> | ||
| <artifactId>maven-surefire-plugin</artifactId> |
There was a problem hiding this comment.
we have no test to run, so this isn't needed/useful.
| <!--This plugin's configuration is used to store Eclipse m2e settings | ||
| only. It has no influence on the Maven build itself. --> | ||
| <plugin> | ||
| <groupId>org.eclipse.m2e</groupId> |
There was a problem hiding this comment.
We can re-add this if a user wants this to run in eclipse, but it was causing issues for us, and none of us use eclipse to benefit from the extra xml.
| <groupId>com.google.api.grpc</groupId> | ||
| <artifactId>proto-google-common-protos</artifactId> | ||
| <version>2.9.6</version> | ||
| <scope>test</scope> |
There was a problem hiding this comment.
no tests, no test dependencies...
|
|
||
| <profiles> | ||
| <profile> | ||
| <id>java-8</id> |
There was a problem hiding this comment.
We can't build arrow 18 with Java 8 anyway.
| with other annotation processors. | ||
| See https://github.com/jbosstools/m2e-apt/issues/62 for details | ||
| --> | ||
| <id>error-prone</id> |
There was a problem hiding this comment.
We aren't writing any java by hand, this extra wiring is unnecessary.
I'll add comments to the PR, but we don't need all of this if it seems like too much, just trying to standardize/cleanup as much as I can without actually changing structure.