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.

Sign up for GitHub

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

Jump to bottom

Return state from update state node wrapper methods #109

Merged
merged 3 commits into from
Sep 25, 2020
Merged

Return state from update state node wrapper methods #109

merged 3 commits into from
Sep 25, 2020

Conversation

mirgee
Copy link
Contributor

@mirgee mirgee commented Sep 24, 2020

Signed-off-by: Miroslav Kovar miroslavkovar@protonmail.com

@mirgee mirgee force-pushed the wrapper-update-state-return-value branch from f1316b0 to adc8653 Compare September 24, 2020 16:23
Patrik-Stas
Patrik-Stas requested changes Sep 25, 2020
View reviewed changes
Copy link
Contributor

@Patrik-Stas Patrik-Stas left a comment

Choose a reason for hiding this comment

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

  1. Can you add assert to a test in wrapper that the return value is the same as getState()?
  2. And on the other hand, possibly improve agent code to utilize update state return value instead of doing explicit getState after update?

@mirgee
Return state from update state node wrapper methods
922f97e 
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
@mirgee
Add tests
17f01ed 
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
@mirgee mirgee force-pushed the wrapper-update-state-return-value branch from adc8653 to 17f01ed Compare September 25, 2020 08:07
@mirgee
Use updateState in agent instead of getState
11917e0 
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
@Patrik-Stas Patrik-Stas merged commit ea331ec into master Sep 25, 2020
@Patrik-Stas Patrik-Stas deleted the wrapper-update-state-return-value branch October 16, 2020 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Patrik-Stas Patrik-Stas Patrik-Stas approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mirgee @Patrik-Stas