adoroszlai edited a comment on pull request #3155:
URL: https://github.com/apache/ozone/pull/3155#issuecomment-1059533909
Good points @errose28.
* I also wanted to suggest using the enum type as parameter instead of
`int`, but we have to consider the "newer than any versions this component is
aware of (i.e. has enum constants for)" case. Do we create a
`FUTURE_VERSIONS(Integer.MAX_VALUE)` enum constant for that case? At least we
should have an overloaded version of the methods:
```
foo(int)
foo(...Version)
```
* I have already found a case where the wrong version is being passed
unintentionally (HDDS-6418).
* Regarding `{Shared,External,Advertised,Broadcasted}DatanodeVersion`: I'm
not sure why plain `DatanodeVersion` would imply that it is internal.
* `ComponentVersion` might be a good name for the common interface.
* The discussion we had about `CURRENT_VERSION` is mostly
implementation-specific. The point is that we should have a constant rather
than a (very slightly expensive) method.
> Having to pass `ClientVersion.CURRENT_VERSION` when constructing protos on
the server side is a bit confusing, but it looks like an unfortunate
consequence of using the same protos for internal processing and client
communication.
It might also indicate a case where client version is ignored (and could
result in compatibility problem).
--
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]
