You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@streams.apache.org by GitBox <gi...@apache.org> on 2022/08/28 19:09:11 UTC
[GitHub] [streams] steveblackmon opened a new pull request, #529: resolves STREAMS-683: upgrade jsonschema2pojo usage to modern version
steveblackmon opened a new pull request, #529:
URL: https://github.com/apache/streams/pull/529
because there were syntax changes in the new version, had to tweak a lot of json schema files and some utility classes
--
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: dev-unsubscribe@streams.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [streams] steveblackmon commented on a diff in pull request #529: resolves STREAMS-683: upgrade jsonschema2pojo usage to modern version
Posted by GitBox <gi...@apache.org>.
steveblackmon commented on code in PR #529:
URL: https://github.com/apache/streams/pull/529#discussion_r970297442
##########
streams-util/src/main/java/org/apache/streams/util/schema/SchemaStoreImpl.java:
##########
@@ -63,7 +63,7 @@ public synchronized Schema create(URI uri) {
if (uri.toString().contains("#") && !uri.toString().endsWith("#")) {
Schema newSchema = new Schema(baseUri, baseNode, null, true);
this.schemas.put(baseUri, newSchema);
- JsonNode childContent = this.fragmentResolver.resolve(baseNode, '#' + StringUtils.substringAfter(uri.toString(), "#"));
+ JsonNode childContent = this.fragmentResolver.resolve(baseNode, '#' + StringUtils.substringAfter(uri.toString(), "#"), "#");
Review Comment:
I'm not entirely sure - I recall I was working through failing builds and/or tests that were repaired with this change.
--
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: dev-unsubscribe@streams.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [streams] steveblackmon commented on a diff in pull request #529: resolves STREAMS-683: upgrade jsonschema2pojo usage to modern version
Posted by GitBox <gi...@apache.org>.
steveblackmon commented on code in PR #529:
URL: https://github.com/apache/streams/pull/529#discussion_r970296499
##########
streams-contrib/streams-provider-instagram/src/main/java/org/apache/streams/instagram/serializer/util/InstagramActivityUtil.java:
##########
@@ -21,18 +21,11 @@
import org.apache.streams.exceptions.ActivityConversionException;
import org.apache.streams.exceptions.ActivitySerializerException;
-import org.apache.streams.instagram.pojo.Comment;
-import org.apache.streams.instagram.pojo.Comments;
-import org.apache.streams.instagram.pojo.Images;
-import org.apache.streams.instagram.pojo.Media;
-import org.apache.streams.instagram.pojo.UserInfo;
-import org.apache.streams.instagram.pojo.UserInfoCounts;
-import org.apache.streams.instagram.pojo.Videos;
+import org.apache.streams.instagram.pojo.*;
Review Comment:
IDE did this. will fix
--
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: dev-unsubscribe@streams.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [streams] steveblackmon commented on a diff in pull request #529: resolves STREAMS-683: upgrade jsonschema2pojo usage to modern version
Posted by GitBox <gi...@apache.org>.
steveblackmon commented on code in PR #529:
URL: https://github.com/apache/streams/pull/529#discussion_r970296414
##########
streams-contrib/streams-provider-instagram/src/main/java/org/apache/streams/instagram/serializer/util/InstagramActivityUtil.java:
##########
@@ -21,18 +21,11 @@
import org.apache.streams.exceptions.ActivityConversionException;
import org.apache.streams.exceptions.ActivitySerializerException;
-import org.apache.streams.instagram.pojo.Comment;
-import org.apache.streams.instagram.pojo.Comments;
-import org.apache.streams.instagram.pojo.Images;
-import org.apache.streams.instagram.pojo.Media;
-import org.apache.streams.instagram.pojo.UserInfo;
-import org.apache.streams.instagram.pojo.UserInfoCounts;
-import org.apache.streams.instagram.pojo.Videos;
+import org.apache.streams.instagram.pojo.*;
import org.apache.streams.pojo.extensions.ExtensionUtil;
import org.apache.streams.pojo.json.Activity;
import org.apache.streams.pojo.json.ActivityObject;
import org.apache.streams.pojo.json.Image;
-import org.apache.streams.pojo.json.ImageParent;
Review Comment:
yes - the prior generated sources was creating this derived class, but improving the schemata for/near MediaItem got rid of 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: dev-unsubscribe@streams.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [streams] steveblackmon commented on a diff in pull request #529: resolves STREAMS-683: upgrade jsonschema2pojo usage to modern version
Posted by GitBox <gi...@apache.org>.
steveblackmon commented on code in PR #529:
URL: https://github.com/apache/streams/pull/529#discussion_r970298516
##########
streams-contrib/streams-provider-linkedin/pom.xml:
##########
@@ -138,36 +138,6 @@
</testResource>
</testResources>
<plugins>
- <plugin>
- <groupId>org.apache.streams.plugins</groupId>
- <artifactId>streams-plugin-pojo</artifactId>
- <version>${project.version}</version>
- <configuration>
- <sourcePaths>
- <sourcePath>${project.basedir}/src/main/jsonschema</sourcePath>
- </sourcePaths>
- <targetDirectory>${project.basedir}/target/generated-sources/pojo</targetDirectory>
- <targetPackage>org.apache.streams.linkedin.pojo</targetPackage>
- </configuration>
- </plugin>
- <plugin>
- <groupId>org.codehaus.mojo</groupId>
Review Comment:
without the pojo plugin activated, the build helper plugin is not needed.
--
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: dev-unsubscribe@streams.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [streams] steveblackmon commented on a diff in pull request #529: resolves STREAMS-683: upgrade jsonschema2pojo usage to modern version
Posted by GitBox <gi...@apache.org>.
steveblackmon commented on code in PR #529:
URL: https://github.com/apache/streams/pull/529#discussion_r970297021
##########
streams-plugins/streams-plugin-pojo/src/main/java/org/apache/streams/plugins/StreamsPojoGenerationConfig.java:
##########
@@ -106,10 +106,10 @@ public boolean isIncludeJsr303Annotations() {
return true;
}
- @Override
- public boolean isUseCommonsLang3() {
- return true;
- }
+// @Override
Review Comment:
older version of abstract parent class from jsonschema2pojo contained this method, but newer version does not. it should be removed as you mentioned.
--
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: dev-unsubscribe@streams.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [streams] steveblackmon merged pull request #529: resolves STREAMS-683: upgrade jsonschema2pojo usage to modern version
Posted by GitBox <gi...@apache.org>.
steveblackmon merged PR #529:
URL: https://github.com/apache/streams/pull/529
--
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: dev-unsubscribe@streams.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [streams] b-hodge commented on a diff in pull request #529: resolves STREAMS-683: upgrade jsonschema2pojo usage to modern version
Posted by GitBox <gi...@apache.org>.
b-hodge commented on code in PR #529:
URL: https://github.com/apache/streams/pull/529#discussion_r970276494
##########
streams-util/src/main/java/org/apache/streams/util/schema/SchemaStoreImpl.java:
##########
@@ -63,7 +63,7 @@ public synchronized Schema create(URI uri) {
if (uri.toString().contains("#") && !uri.toString().endsWith("#")) {
Schema newSchema = new Schema(baseUri, baseNode, null, true);
this.schemas.put(baseUri, newSchema);
- JsonNode childContent = this.fragmentResolver.resolve(baseNode, '#' + StringUtils.substringAfter(uri.toString(), "#"));
+ JsonNode childContent = this.fragmentResolver.resolve(baseNode, '#' + StringUtils.substringAfter(uri.toString(), "#"), "#");
Review Comment:
What's going on here? Is this change related to the `jsonschema2pojo` version bump, or something else?
##########
streams-contrib/streams-provider-instagram/src/main/java/org/apache/streams/instagram/serializer/util/InstagramActivityUtil.java:
##########
@@ -21,18 +21,11 @@
import org.apache.streams.exceptions.ActivityConversionException;
import org.apache.streams.exceptions.ActivitySerializerException;
-import org.apache.streams.instagram.pojo.Comment;
-import org.apache.streams.instagram.pojo.Comments;
-import org.apache.streams.instagram.pojo.Images;
-import org.apache.streams.instagram.pojo.Media;
-import org.apache.streams.instagram.pojo.UserInfo;
-import org.apache.streams.instagram.pojo.UserInfoCounts;
-import org.apache.streams.instagram.pojo.Videos;
+import org.apache.streams.instagram.pojo.*;
Review Comment:
What's the reason for not continuing to enumerate imports of specific classes? Are there possible future cases where importing all classes matching the wildcard would be incorrect or dangerous?
##########
streams-contrib/streams-provider-instagram/src/main/java/org/apache/streams/instagram/serializer/util/InstagramActivityUtil.java:
##########
@@ -21,18 +21,11 @@
import org.apache.streams.exceptions.ActivityConversionException;
import org.apache.streams.exceptions.ActivitySerializerException;
-import org.apache.streams.instagram.pojo.Comment;
-import org.apache.streams.instagram.pojo.Comments;
-import org.apache.streams.instagram.pojo.Images;
-import org.apache.streams.instagram.pojo.Media;
-import org.apache.streams.instagram.pojo.UserInfo;
-import org.apache.streams.instagram.pojo.UserInfoCounts;
-import org.apache.streams.instagram.pojo.Videos;
+import org.apache.streams.instagram.pojo.*;
import org.apache.streams.pojo.extensions.ExtensionUtil;
import org.apache.streams.pojo.json.Activity;
import org.apache.streams.pojo.json.ActivityObject;
import org.apache.streams.pojo.json.Image;
-import org.apache.streams.pojo.json.ImageParent;
Review Comment:
How does this change relate to the version upgrade of `jsonschema2pojo` dependency?
I can't find this class anywhere in the `streams` repo, but also don't see evidence of it being removed in this PR. Was this dependency coming from the `org.apache.streams.plugins_streams-plugin-pojo` Maven dependency that's also being removed?
##########
streams-contrib/streams-provider-linkedin/pom.xml:
##########
@@ -138,36 +138,6 @@
</testResource>
</testResources>
<plugins>
- <plugin>
- <groupId>org.apache.streams.plugins</groupId>
- <artifactId>streams-plugin-pojo</artifactId>
- <version>${project.version}</version>
- <configuration>
- <sourcePaths>
- <sourcePath>${project.basedir}/src/main/jsonschema</sourcePath>
- </sourcePaths>
- <targetDirectory>${project.basedir}/target/generated-sources/pojo</targetDirectory>
- <targetPackage>org.apache.streams.linkedin.pojo</targetPackage>
- </configuration>
- </plugin>
- <plugin>
- <groupId>org.codehaus.mojo</groupId>
Review Comment:
What's the reason for removing this dependency?
##########
streams-contrib/streams-provider-linkedin/pom.xml:
##########
@@ -138,36 +138,6 @@
</testResource>
</testResources>
<plugins>
- <plugin>
- <groupId>org.apache.streams.plugins</groupId>
Review Comment:
What's the reason for removing this dependency?
##########
streams-plugins/streams-plugin-pojo/src/main/java/org/apache/streams/plugins/StreamsPojoGenerationConfig.java:
##########
@@ -106,10 +106,10 @@ public boolean isIncludeJsr303Annotations() {
return true;
}
- @Override
- public boolean isUseCommonsLang3() {
- return true;
- }
+// @Override
Review Comment:
1. What _was_ this function being used for? I don't see any references to it elsewhere in the repo.
2. Why comment-out instead of just removing the function entirely?
--
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: dev-unsubscribe@streams.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [streams] steveblackmon commented on a diff in pull request #529: resolves STREAMS-683: upgrade jsonschema2pojo usage to modern version
Posted by GitBox <gi...@apache.org>.
steveblackmon commented on code in PR #529:
URL: https://github.com/apache/streams/pull/529#discussion_r970298388
##########
streams-contrib/streams-provider-linkedin/pom.xml:
##########
@@ -138,36 +138,6 @@
</testResource>
</testResources>
<plugins>
- <plugin>
- <groupId>org.apache.streams.plugins</groupId>
Review Comment:
this plugin was added/activated in this module via copy/paste early on. the upgraded plugin is less willing to overlook being activated when there are no schemas to generate from, so i took it out.
--
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: dev-unsubscribe@streams.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org