-
Notifications
You must be signed in to change notification settings - Fork 1.3k
DATAES-976 - Implement CrudRepository.delete(Iterable<ID> ids). #554
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
Conversation
@@ -107,7 +109,7 @@ public void saveShouldSaveSingleEntity() { | |||
public void saveShouldComputeMultipleEntities() { | |||
|
|||
repository | |||
.saveAll(Arrays.asList(SampleEntity.builder().build(), SampleEntity.builder().build(), | |||
.saveAll(asList(SampleEntity.builder().build(), SampleEntity.builder().build(), |
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.
Nit: We generally don't import Arrays
statically to avoid clashes.
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.
Hmm, I only knew the rule that static imports are ok in tests. Will undo
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.
I reverted those changes.
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.
Looks good. One thing that I noticed is that deleteAll(…)
doesn't seem to make use of entity callbacks or lifecycle events @sothawo.
…teAllById(Iterable<ID> ids). Original pull request: #554.
Use deleteAllById(…) from deleteAll(…). Simplify implementation in SimpleReactiveElasticsearchRepository. Original pull request: #554.
That's merged and polished now. |
Merge directly after DATACMNS-800