gnodet commented on code in PR #22304:
URL: https://github.com/apache/camel/pull/22304#discussion_r3005060099
##########
dsl/camel-yaml-dsl/camel-yaml-dsl/src/generated/resources/schema/camelYamlDsl.json:
##########
@@ -6110,21 +6099,50 @@
}
} ]
},
+ "org.apache.camel.model.SagaActionUriDefinition" : {
Review Comment:
**Blocking**: This generated file contains ~1,500 lines of changes that are
**not part of the S1612 lambda-to-method-reference refactoring**:
- Removal of `groovyJson` / `GroovyJSonDataFormat` entries (3 locations)
- Addition of `SagaActionUriDefinition` (new schema definition, shown here)
- `SagaDefinition.compensation` and `completion` changed from `type: string`
to `$ref: SagaActionUriDefinition`
- Addition of `org.apache.camel.model.app.*` and
`org.apache.camel.model.cloud.*` definitions
- `WireTapDefinition` schema expansion/rewrite
These are regeneration artifacts from building against a branch state that
has diverged from `main`. Including unrelated generated file changes in a
focused refactoring PR makes review harder and risks sneaking in unintended
schema changes.
Please revert this file:
```bash
git checkout main --
dsl/camel-yaml-dsl/camel-yaml-dsl/src/generated/resources/schema/camelYamlDsl.json
```
Or regenerate it cleanly against the PR's target branch.
##########
components/camel-zookeeper-master/src/main/java/org/apache/camel/component/zookeepermaster/group/internal/ZooKeeperGroup.java:
##########
@@ -468,7 +468,7 @@ void refresh(final RefreshMode mode) throws Exception {
try {
ensurePath.ensure(client.getZookeeperClient());
List<String> children =
client.getChildren().usingWatcher(childrenWatcher).forPath(path);
- children.sort((String left, String right) ->
left.compareTo(right));
+ children.sort(Comparable::compareTo);
Review Comment:
Minor: `Comparable::compareTo` uses a raw type method reference. Since
`children` is `List<String>`, `String::compareTo` is more precise and avoids
potential unchecked/raw type warnings.
```suggestion
children.sort(String::compareTo);
```
##########
components/camel-kafka/src/test/java/org/apache/camel/component/kafka/integration/KafkaBreakOnFirstErrorWithBatchUsingKafkaManualCommitIT.java:
##########
@@ -118,12 +118,7 @@ public void configure() {
// adding error message to end
// so we can account for it
.to(to)
- .process(exchange -> {
- // if we don't commit
- // camel will continuously
- // retry the message with an error
- doCommitOffset(exchange);
- });
+
.process(KafkaBreakOnFirstErrorWithBatchUsingKafkaManualCommitIT.this::doCommitOffset);
Review Comment:
Nit: The lambda-to-method-reference conversion here also removed an
explanatory comment that documented *why* the manual commit is needed:
```
// if we don't commit
// camel will continuously
// retry the message with an error
```
Similar comment removals occur in:
- `ElasticsearchRestClientComponentIT.java` ("Assert that the
direct:get-by-id endpoint received the exception")
- `KafkaBatchingProcessingBreakOnFirstErrorManualCommitIT.java` ("Manual
commit on success")
These comments explain *intent*, not implementation, and remain valuable
even with method references. Consider preserving them above the `.process()`
call:
```java
// if we don't commit, camel will continuously retry
the message with an error
.process(KafkaBreakOnFirstErrorWithBatchUsingKafkaManualCommitIT.this::doCommitOffset);
```
--
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]
