gortiz commented on code in PR #11753:
URL: https://github.com/apache/pinot/pull/11753#discussion_r1366552207
##########
pom.xml:
##########
@@ -1227,6 +1232,11 @@
<artifactId>grpc-stub</artifactId>
<version>${grpc.version}</version>
</dependency>
+ <dependency>
+ <groupId>com.google.api.grpc</groupId>
+ <artifactId>proto-google-common-protos</artifactId>
+ <version>2.9.6</version>
Review Comment:
I don't understand the advantage of adding variables for these kind of
things. That variable should not be read anywhere else, so there is no DRY. It
just make the code more difficult to read given you have to move _one thousand_
(!!!) lines above/below to actually know what the variable does. Also, it
increase the chance of having a (honestly simple to resolve) conflict in PRs,
given that we usually add these variables at the end of the properties section.
It would make sense if other projects would use that variable, but that is
not the case given we use the value to define a dependencymanagement, whose
objective is precisely that subprojects do not write the version by their own.
Anyway, it is not the point of this PR, so I'm changing it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
