Copilot commented on code in PR #8046:
URL: https://github.com/apache/incubator-seata/pull/8046#discussion_r3079056629
##########
json-common/pom.xml:
##########
@@ -27,38 +27,25 @@
</parent>
<modelVersion>4.0.0</modelVersion>
<artifactId>json-common</artifactId>
- <packaging>jar</packaging>
+ <packaging>pom</packaging>
<name>json-common ${project.version}</name>
- <description>jsonUtil for Seata modules</description>
-
- <dependencies>
- <dependency>
- <groupId>${project.groupId}</groupId>
- <artifactId>seata-common</artifactId>
- <version>${project.version}</version>
- </dependency>
- <dependency>
- <groupId>${project.groupId}</groupId>
- <artifactId>seata-core</artifactId>
- <version>${project.version}</version>
- </dependency>
- <dependency>
- <groupId>com.alibaba</groupId>
- <artifactId>fastjson</artifactId>
- <scope>provided</scope>
- </dependency>
- <dependency>
- <groupId>com.fasterxml.jackson.core</groupId>
- <artifactId>jackson-databind</artifactId>
- <scope>provided</scope>
- </dependency>
- <dependency>
- <groupId>com.google.code.gson</groupId>
- <artifactId>gson</artifactId>
- <scope>provided</scope>
- </dependency>
- </dependencies>
-
-
-
-</project>
\ No newline at end of file
+ <description>json-common top parent for Seata built with
Maven</description>
+
+ <modules>
+ <module>json-common-core</module>
+ </modules>
+
Review Comment:
Changing `json-common` from `jar` to `pom` makes
`org.apache.seata:json-common` stop producing classes, which is a breaking
change for any external consumers still depending on that artifact. If backward
compatibility is required, consider keeping `json-common` as the jar (e.g., by
introducing a new parent/aggregator artifactId for the multi-module build) or
providing a compatibility/relocation strategy clearly documented for downstream
users.
##########
seata-spring-autoconfigure/seata-spring-autoconfigure-client/src/main/resources/META-INF/additional-spring-configuration-metadata.json:
##########
@@ -372,8 +372,14 @@
"value": "fastjson",
"description": "the default parser."
},
+ {
+ "value": "fastjson2"
+ },
{
"value": "jackson"
+ },
+ {
+ "value": "jackson3"
}
Review Comment:
The newly added enum values (`fastjson2`, `jackson3`) are missing
descriptions in the Spring configuration metadata. Adding a short description
for these values improves IDE autocompletion/tooling help, consistent with the
existing entries that describe the default option.
##########
common/src/main/java/org/apache/seata/common/Constants.java:
##########
@@ -219,6 +219,16 @@ public interface Constants {
*/
String JACKSON_JSON_PARSER_NAME = "jackson";
+ /**
+ * The constant FASTJSON2_JSON_PARSER_NAME
+ */
+ String FASTJSON2_JSON_PARSER_NAME = "fastjson2";
+
+ /**
+ * The constant JACKSON3_JSON_PARSER_NAME
+ */
+ String JACKSON3_JSON_PARSER_NAME = "jackson3";
+
Review Comment:
New public constants `FASTJSON2_JSON_PARSER_NAME` and
`JACKSON3_JSON_PARSER_NAME` were added here, but
`common/src/test/java/.../ConstantsTest.java` currently asserts only the older
parser-name constants. Consider adding assertions for the new constants as well
to prevent accidental regressions.
--
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]
