Copilot commented on code in PR #4178:
URL:
https://github.com/apache/incubator-kie-kogito-runtimes/pull/4178#discussion_r2753006312
##########
quarkus/addons/asyncapi/deployment/src/main/java/org/kie/kogito/quarkus/serverless/workflow/asyncapi/AsyncAPIInfoConverter.java:
##########
@@ -1,3 +1,4 @@
+
Review Comment:
This file now starts with a blank line before the ASF license header. With
the repo’s `license-maven-plugin` configuration
(kogito-build/kogito-build-no-bom-parent/pom.xml), headers are typically
expected at the very beginning of the file; the leading blank line can cause
license checks to fail. Remove the initial empty line so the header begins on
line 1.
```suggestion
```
##########
kogito-serverless-workflow/kogito-serverless-workflow-executor-kafka/src/test/java/org/kie/kogito/serverless/workflow/executor/MockKafkaEventEmitterFactory.java:
##########
@@ -28,7 +28,8 @@
public class MockKafkaEventEmitterFactory extends KafkaEventEmitterFactory {
- public static MockProducer<byte[], CloudEvent> producer = new
MockProducer<>(true, new ByteArraySerializer(), new CloudEventSerializer() {
+ // null key serializer is used because this mock producer doesn't
serialize keys, only values (CloudEvents)
Review Comment:
The added comment says a “null key serializer” is being used, but the key
serializer passed here is `ByteArraySerializer`; the `null` is the additional
constructor argument. Please adjust the comment to describe the actual
parameter/behavior (or drop it) so it doesn’t mislead future readers.
```suggestion
// The mock producer uses ByteArraySerializer for keys and
CloudEventSerializer for values; tests ignore keys and operate only on
CloudEvent values.
```
##########
quarkus/extensions/kogito-quarkus-serverless-workflow-extension/kogito-quarkus-serverless-workflow-integration-test/src/main/resources/specs/asyncAPI.yaml:
##########
@@ -17,34 +17,47 @@
# under the License.
#
-asyncapi: '2.0.0'
-id: 'oneConsumer'
+# Quarkus 3.27.2 upgrade: Migrated from AsyncAPI 2.6.0 to 3.0.0
Review Comment:
The new migration comment says “Migrated from AsyncAPI 2.6.0 to 3.0.0”, but
this file previously declared `asyncapi: '2.0.0'`. Consider updating the
comment to reflect the spec version actually being migrated (e.g., 2.0.0 →
3.0.0, or 2.x → 3.0.0) to avoid confusion.
```suggestion
# Quarkus 3.27.2 upgrade: Migrated from AsyncAPI 2.0.0 to 3.0.0
```
##########
kogito-serverless-workflow/kogito-serverless-workflow-executor-tests/src/test/java/org/kie/kogito/serverless/workflow/executor/MockKafkaEventEmitterFactory.java:
##########
@@ -28,7 +28,8 @@
public class MockKafkaEventEmitterFactory extends KafkaEventEmitterFactory {
- public static MockProducer<byte[], CloudEvent> producer = new
MockProducer<>(true, new ByteArraySerializer(), new CloudEventSerializer() {
+ // null key serializer is used because this mock producer doesn't
serialize keys, only values (CloudEvents)
Review Comment:
The added comment says a “null key serializer” is being used, but the key
serializer passed here is `ByteArraySerializer`; the `null` is the additional
constructor argument. Please adjust the comment to describe the actual
parameter/behavior (or drop it) so it doesn’t mislead future readers.
```suggestion
// null transactional id is used; ByteArraySerializer is used for keys
and CloudEventSerializer for values
```
##########
quarkus/extensions/kogito-quarkus-serverless-workflow-extension/kogito-quarkus-serverless-workflow-integration-test/src/main/resources/specs/callbackResults.yaml:
##########
@@ -17,57 +17,91 @@
# under the License.
#
-asyncapi: '2.0.0'
-id: 'callbackResults'
+# Quarkus 3.27.2 upgrade: Migrated from AsyncAPI 2.6.0 to 3.0.0
Review Comment:
The new migration comment says “Migrated from AsyncAPI 2.6.0 to 3.0.0”, but
this file previously declared `asyncapi: '2.0.0'`. Consider updating the
comment to reflect the spec version actually being migrated (e.g., 2.0.0 →
3.0.0, or 2.x → 3.0.0) to avoid confusion.
```suggestion
# Quarkus 3.27.2 upgrade: Migrated from AsyncAPI 2.0.0 to 3.0.0
```
##########
quarkus/bom/pom.xml:
##########
@@ -38,7 +38,7 @@
<properties>
<!-- Keep it aligned with
https://github.com/quarkusio/quarkus/blob/3.20.1/pom.xml#L72 -->
Review Comment:
The inline comment still references the Quarkus 3.20.1 BOM URL, but this PR
upgrades Quarkus-related deps. Please update the referenced Quarkus
version/link (or make it version-agnostic) so future maintainers can verify
alignment correctly.
```suggestion
<!-- Keep it aligned with
https://github.com/quarkusio/quarkus/blob/main/pom.xml#L72 -->
```
##########
quarkus/extensions/kogito-quarkus-serverless-workflow-extension/kogito-quarkus-serverless-workflow-extension-live-reload-test/src/main/resources/specs/asyncAPI.yaml:
##########
@@ -17,34 +17,47 @@
# under the License.
#
-asyncapi: '2.0.0'
-id: 'oneConsumer'
+# Quarkus 3.27.2 upgrade: Migrated from AsyncAPI 2.6.0 to 3.0.0
Review Comment:
The new migration comment says “Migrated from AsyncAPI 2.6.0 to 3.0.0”, but
this file previously declared `asyncapi: '2.0.0'`. Consider updating the
comment to reflect the spec version actually being migrated (e.g., 2.0.0 →
3.0.0, or 2.x → 3.0.0) to avoid confusion.
```suggestion
# Quarkus 3.27.2 upgrade: Migrated from AsyncAPI 2.0.0 to 3.0.0
```
--
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]