-
Notifications
You must be signed in to change notification settings - Fork 562
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(client): provide the Java client's interfaces for migrating a process instance #15199
feat(client): provide the Java client's interfaces for migrating a process instance #15199
Conversation
...va/src/main/java/io/camunda/zeebe/client/api/command/MigrateProcessInstanceCommandStep1.java
Dismissed
Show dismissed
Hide dismissed
...va/src/main/java/io/camunda/zeebe/client/api/command/MigrateProcessInstanceCommandStep1.java
Fixed
Show fixed
Hide fixed
...va/src/main/java/io/camunda/zeebe/client/api/command/MigrateProcessInstanceCommandStep1.java
Dismissed
Show dismissed
Hide dismissed
...va/src/main/java/io/camunda/zeebe/client/api/command/MigrateProcessInstanceCommandStep1.java
Dismissed
Show dismissed
Hide dismissed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @berkaycanbc 🙌
👍 I really like the examples in the command method's JavaDoc. They inspire me to ask for more of these in other places.
❌ There are a few small things that I believe should be changed.
🔧 I also made some suggestions. Please have a look.
clients/java/src/main/java/io/camunda/zeebe/client/ZeebeClient.java
Outdated
Show resolved
Hide resolved
clients/java/src/main/java/io/camunda/zeebe/client/ZeebeClient.java
Outdated
Show resolved
Hide resolved
clients/java/src/main/java/io/camunda/zeebe/client/ZeebeClient.java
Outdated
Show resolved
Hide resolved
...va/src/main/java/io/camunda/zeebe/client/api/command/MigrateProcessInstanceCommandStep1.java
Show resolved
Hide resolved
...va/src/main/java/io/camunda/zeebe/client/api/command/MigrateProcessInstanceCommandStep1.java
Outdated
Show resolved
Hide resolved
clients/java/src/main/java/io/camunda/zeebe/client/api/command/MigrationPlan.java
Outdated
Show resolved
Hide resolved
...va/src/main/java/io/camunda/zeebe/client/impl/command/MigrateProcessInstanceCommandImpl.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great feedback @korthout, thanks! 😍
I sent changes based on your comments. 👍
...va/src/main/java/io/camunda/zeebe/client/api/command/MigrateProcessInstanceCommandStep1.java
Dismissed
Show dismissed
Hide dismissed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes @berkaycanbc ❤️
I have a few last suggestions, but nothing major. Please have a look.
LGTM 👍
clients/java/src/main/java/io/camunda/zeebe/client/ZeebeClient.java
Outdated
Show resolved
Hide resolved
...va/src/main/java/io/camunda/zeebe/client/api/command/MigrateProcessInstanceCommandStep1.java
Outdated
Show resolved
Hide resolved
clients/java/src/main/java/io/camunda/zeebe/client/api/command/MigrationPlan.java
Outdated
Show resolved
Hide resolved
2915c67
to
1be8626
Compare
A MigrationPlan object and its builder is added. The MigrationPlan object can be reused to migrate different process instances with the same active elements that needs to be migrated to the same process definition.
Steps are structured in a way that they force users to follow required order. E.g. targetProcessDefinitionKey needs to be provided before mappingInstructions. Also, addMappingInstruction method is duplicated in both step 2 and final step. This is because we still want to be able to add intermediate steps to the command chain using step 2 interface.
…gration The implementation of process instance migration command left unimplemented because no actual implementation is available in the engine at the moment.
Tests only verify the command chain at the moment. Later will be updated once actual implementation takes place in the engine.
1be8626
to
8a5e443
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the more detailed commits ❤️
LGTM 👍
bors merge |
Build succeeded: |
8b70dcd
Description
Related issues
closes #15161
Definition of Done
Not all items need to be done depending on the issue and the pull request.
Code changes:
backport stable/1.3
) to the PR, in case that fails you need to create backports manually.Testing:
Documentation:
Other teams:
If the change impacts another team an issue has been created for this team, explaining what they need to do to support this change.
Please refer to our review guidelines.