Skip to content
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

Resolve missing internal state update in EventSourcedRememberEntities… #30217

Merged

Conversation

jneumaier
Copy link
Contributor

See #30215

@lightbend-cla-validator
Copy link

Hi @jneumaier,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

https://www.lightbend.com/contribute/cla

@akka-ci
Copy link

akka-ci commented May 3, 2021

Thank you for your pull request! After a quick sanity check one of the team will reply with 'OK ΤO ΤESΤ' to kick off our automated validation on Jenkins. This compiles the project, runs the tests, and checks for things like binary compatibility and source code formatting. When two team members have also manually reviewed and (perhaps after asking for some amendments) accepted your contribution, it should be good to be merged.

For more details about our contributing process, check out CONTRIBUTING.md - and feel free to ask!

@johanandren
Copy link
Member

OK TO TEST

@akka-ci akka-ci added validating PR is currently being validated by Jenkins needs-attention Indicates a PR validation failure (set by CI infrastructure) and removed validating PR is currently being validated by Jenkins labels May 3, 2021
@akka-ci
Copy link

akka-ci commented May 3, 2021

Test FAILed.

Copy link
Member

@johanandren johanandren left a comment

Choose a reason for hiding this comment

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

Good catch, curious that nobody noticed earlier.

@akka-ci akka-ci added validating PR is currently being validated by Jenkins needs-attention Indicates a PR validation failure (set by CI infrastructure) and removed needs-attention Indicates a PR validation failure (set by CI infrastructure) validating PR is currently being validated by Jenkins labels May 3, 2021
@akka-ci
Copy link

akka-ci commented May 3, 2021

Test FAILed.

@akka-ci akka-ci added validating PR is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed needs-attention Indicates a PR validation failure (set by CI infrastructure) validating PR is currently being validated by Jenkins labels May 3, 2021
@akka-ci
Copy link

akka-ci commented May 3, 2021

Test PASSed.

@jneumaier
Copy link
Contributor Author

@chbatey You also contributed to EventSourcedRememberEntitiesShardStore, maybe you can do a review?

Copy link
Member

@octonato octonato left a comment

Choose a reason for hiding this comment

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

LGTM

@johanandren johanandren merged commit 73d74df into akka:master May 4, 2021
Copy link
Member

@patriknw patriknw left a comment

Choose a reason for hiding this comment

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

thanks for fixing

@@ -101,7 +101,7 @@ private[akka] final class EventSourcedRememberEntitiesShardStore(
left -= 1
if (left == 0) {
sender() ! RememberEntitiesShardStore.UpdateDone(started, stopped)
state.copy(state.entities.union(started).diff(stopped))
state = state.copy(state.entities.union(started).diff(stopped))
Copy link
Member

Choose a reason for hiding this comment

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

😱

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tested PR that was successfully built and tested by Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants