Skip to content

Conversation

@rjrudin
Copy link
Contributor

@rjrudin rjrudin commented Dec 30, 2025

Just some cleanup here before new functionality is added. Fixing some warnings in WriteBatcherImpl, and changed BatchWriter into a record.

Copilot AI review requested due to automatic review settings December 30, 2025 18:30
@github-actions
Copy link

github-actions bot commented Dec 30, 2025

Copyright Validation Results
Total: 5 | Passed: 4 | Failed: 0 | Skipped: 1 | at: 2025-12-30 18:39:41 UTC | commit: f65fcf8

⏭️ Skipped (Excluded) Files

  • gradle.properties

✅ Valid Files

  • marklogic-client-api/src/main/java/com/marklogic/client/datamovement/WriteBatcher.java
  • marklogic-client-api/src/main/java/com/marklogic/client/datamovement/impl/BatchWriter.java
  • marklogic-client-api/src/main/java/com/marklogic/client/datamovement/impl/WriteBatcherImpl.java
  • marklogic-client-api/src/test/java/com/marklogic/client/test/AbstractClientTest.java

✅ All files have valid copyright headers!

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR performs refactoring work on the WriteBatcher implementation to prepare for upcoming incremental write functionality. The changes focus on code cleanup, modernizing Java syntax, and improving code organization without altering core functionality.

Key Changes:

  • Converted BatchWriter from a class to a Java record for cleaner immutability
  • Modernized code style by simplifying boolean comparisons and using pattern matching
  • Consolidated import statements for better organization

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
marklogic-client-api/src/main/java/com/marklogic/client/datamovement/impl/WriteBatcherImpl.java Simplified boolean comparisons, consolidated imports, removed unused message parameter from error handling
marklogic-client-api/src/main/java/com/marklogic/client/datamovement/impl/BatchWriter.java Converted class to record, refactored run() method into smaller methods, used pattern matching for instanceof checks
marklogic-client-api/src/main/java/com/marklogic/client/datamovement/WriteBatcher.java Removed commented-out code, cleaned up method modifiers, consolidated imports
marklogic-client-api/src/test/java/com/marklogic/client/test/AbstractClientTest.java Added AfterEach hook to properly release client resources after tests
gradle.properties Bumped version from 8.0-SNAPSHOT to 8.1-SNAPSHOT

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


class BatchWriter implements Runnable {
record BatchWriter(BatchWriteSet batchWriteSet) implements Runnable {

Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation check for empty batch has been removed during the conversion to a record. Previously, the constructor threw an IllegalStateException if 'batchWriteSet.getDocumentWriteSet().size() == 0'. This validation should be added to a compact constructor in the record to maintain the same safety guarantee.

Suggested change
BatchWriter {
if (batchWriteSet.getDocumentWriteSet().size() == 0) {
throw new IllegalStateException("BatchWriter requires a non-empty DocumentWriteSet.");
}
}

Copilot uses AI. Check for mistakes.
Just some cleanup here before new functionality is added. Fixing some warnings in WriteBatcherImpl, and changed BatchWriter into a record.
@rjrudin rjrudin force-pushed the feature/26420-refactor branch from 45237f1 to f65fcf8 Compare December 30, 2025 18:39
@rjrudin rjrudin merged commit b452860 into develop Dec 30, 2025
3 checks passed
@rjrudin rjrudin deleted the feature/26420-refactor branch December 30, 2025 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants