sashapolo commented on code in PR #1116:
URL: https://github.com/apache/ignite-3/pull/1116#discussion_r978482246
##########
modules/rest-api/src/main/java/org/apache/ignite/internal/rest/api/cluster/IgniteProductVersionDto.java:
##########
@@ -38,26 +39,28 @@ public class IgniteProductVersionDto {
/** Maintenance version number. */
private final short maintenance;
- /** Flag indicating if this is a snapshot release. */
- private final boolean isSnapshot;
+ /** Patch version number. */
Review Comment:
I understand that this is not related to this PR, but why do we need this
class? Can we use the String representation instead?
##########
modules/core/src/main/java/org/apache/ignite/internal/properties/IgniteProductVersion.java:
##########
@@ -109,41 +112,48 @@ public byte maintenance() {
}
/**
- * Returns {@code true} if this is a snapshot release, {@code false}
otherwise.
+ * Returns the patch version number.
*/
- public boolean snapshot() {
- return isSnapshot;
+ public @Nullable Byte patch() {
+ return patch;
}
/**
- * Returns the alpha version of this release or an empty string if this is
not an alpha release.
+ * Returns the pre-release version.
Review Comment:
Same here
##########
modules/core/src/main/java/org/apache/ignite/internal/properties/IgniteProductVersion.java:
##########
@@ -29,11 +30,9 @@
*/
public class IgniteProductVersion implements Serializable {
private static final Pattern VERSION_PATTERN =
-
Pattern.compile("(?<major>\\d+)\\.(?<minor>\\d+)\\.(?<maintenance>\\d+)((?<snapshot>-SNAPSHOT)|-(?<alpha>alpha\\d+))?");
+
Pattern.compile("(?<major>\\d+)\\.(?<minor>\\d+)\\.(?<maintenance>\\d+)(\\.(?<patch>\\d+))?(-(?<preRelease>[0-9A-Za-z]+))?");
Review Comment:
Can you please add a description about the format to the javadoc? E.g. what
`preRelease` is and some examples for Ignite versions?
##########
modules/core/src/main/java/org/apache/ignite/internal/properties/IgniteProductVersion.java:
##########
@@ -109,41 +112,48 @@ public byte maintenance() {
}
/**
- * Returns {@code true} if this is a snapshot release, {@code false}
otherwise.
+ * Returns the patch version number.
Review Comment:
Please add a description when this method can return `{@code null}`
##########
modules/core/src/main/java/org/apache/ignite/internal/properties/IgniteProductVersion.java:
##########
@@ -29,11 +30,9 @@
*/
public class IgniteProductVersion implements Serializable {
private static final Pattern VERSION_PATTERN =
-
Pattern.compile("(?<major>\\d+)\\.(?<minor>\\d+)\\.(?<maintenance>\\d+)((?<snapshot>-SNAPSHOT)|-(?<alpha>alpha\\d+))?");
+
Pattern.compile("(?<major>\\d+)\\.(?<minor>\\d+)\\.(?<maintenance>\\d+)(\\.(?<patch>\\d+))?(-(?<preRelease>[0-9A-Za-z]+))?");
Review Comment:
it's better to use non-capturing groups, e.g.: `(?:\\.(?<patch>\\d+))`
##########
modules/core/src/main/java/org/apache/ignite/internal/properties/IgniteProductVersion.java:
##########
@@ -29,11 +30,9 @@
*/
public class IgniteProductVersion implements Serializable {
private static final Pattern VERSION_PATTERN =
-
Pattern.compile("(?<major>\\d+)\\.(?<minor>\\d+)\\.(?<maintenance>\\d+)((?<snapshot>-SNAPSHOT)|-(?<alpha>alpha\\d+))?");
+
Pattern.compile("(?<major>\\d+)\\.(?<minor>\\d+)\\.(?<maintenance>\\d+)(\\.(?<patch>\\d+))?(-(?<preRelease>[0-9A-Za-z]+))?");
Review Comment:
Can we replace `[0-9A-Za-z]` with `\w`?
--
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]
