V3 casl 1466 v3 mom 10 #538
Conversation
Signed-off-by: Jakub Amanowicz <[email protected]>
Signed-off-by: Jakub Amanowicz <[email protected]>
Marcin-Here
left a comment
Marcin-Here
left a comment
There was a problem hiding this comment.
🥇 good job! few minor comments.
here-naksha-lib-mom10/README.md
Outdated
|
|
||
| #### Populating old delta | ||
|
|
||
| | `@ns:com:here:mom:delta` properties supported in Naksha V2 (pre MOM 10) | equivalents in `meta.moderationInfo` (MOM 10+) | |
here-naksha-lib-mom10/README.md
Outdated
| - we return feature in the same shape as it was given to Naksha in the writing phase | ||
|
|
||
| The class responsible for these operations | ||
| is [Mom10Transformation](../java/com/here/naksha/mom10/transform/Mom10Transformation.java). |
There was a problem hiding this comment.
I think "src/jvmMain/java/com/here/naksha/mom10/Mom10Transformation.java" this would be working path for Mom10Transformation
| } | ||
|
|
||
| public static void dropPreMom10Namespaces(@Nullable NakshaFeature feature) { | ||
| NakshaProperties properties = feature.getProperties(); |
There was a problem hiding this comment.
If feature is Nullable, shouldn't we guard for that before calling .getProperties()?
There was a problem hiding this comment.
We definitely should! Fixed :)
|
|
||
| public class Mom10Verification { | ||
|
|
||
| private static final Pattern SHORT_SEM_VER = Pattern.compile("^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\..+$"); |
| } | ||
|
|
||
| private static @Nullable Integer getMajorFrom(@NotNull String modelVersion) { | ||
| Matcher matcher = SHORT_SEM_VER.matcher(modelVersion); |
There was a problem hiding this comment.
In tests we have case as ""8.91" so if this is possible version, then let's say "10.0" is also possible and if so then our regex "\..+$" would return false on it. Is this scenario possible?
There was a problem hiding this comment.
It is possible and we treat "10.0" as invalid value, "10.0.0" is ok though
There was a problem hiding this comment.
There was a mistake in tests though, fixed
| } | ||
|
|
||
| private static Stream<Named<VerificationCase>> shouldVerifyIfFeatureIsInMom10() { | ||
| return Stream.of( |
There was a problem hiding this comment.
if above comment about "10.0" scenario is possible, let's add tests for it. If no, ignore this comment.
Signed-off-by: Jakub Amanowicz <[email protected]>
2 participants
No description provided.