zentol commented on a change in pull request #18381:
URL: https://github.com/apache/flink/pull/18381#discussion_r787702215
##########
File path: flink-annotations/src/main/java/org/apache/flink/FlinkVersion.java
##########
@@ -69,9 +71,14 @@ public boolean isNewerVersionThan(FlinkVersion otherVersion)
{
}
/** Returns all versions equal to or higher than the selected version. */
- public Set<FlinkVersion> orHigher() {
+ public Set<FlinkVersion> orHigherMinusSnapshot() {
Review comment:
If anything the enum should have a boolean flag, but it seems somewhat
overly complicated and isn't really intuitive.
For example, let's say we add this with the flag to 1.15. In the 1.15.0
release the `1_15` enum would still be marked as "not released", because if it
were marked as released then the compatibility tests would complain. So we have
release that considers itself as not released.
We also end up with _additional_ manual steps on _both_ the `master` and
`release-1.15` branch, as they have to set this flag to true after 1.15.0 is
released. But only after someone actually went through the trouble of creating
the snapshots for the compatibility tests. And if we add additional table api
compatibility tests, then this step just got worse.
I'm wondering if we shouldn't separate this distinction from the enum itself.
For example, there could be a flag (or separate ones per test) in the
compatibility tests for whether they should also cover the latest version.
Or they could just ignore versions for which the files don't exist. As is we
could never drop the 1.3 snapshots, because the enum still claims 1.3 exists.
Although this adds a risk that the tests are completely broken and we don't
notice it.
We could just introduce a separate enum/set for which Flink versions are
relevant for compatibility tests. I think I'd prefer this option.
--
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]
