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

Remove redundant state existence check when loading inputs/outputs #244

Merged
merged 1 commit into from
Apr 19, 2024
Merged

Remove redundant state existence check when loading inputs/outputs #244

merged 1 commit into from
Apr 19, 2024

Conversation

whfuyn
Copy link
Contributor

@whfuyn whfuyn commented Apr 18, 2024

.contains_key(key) is equivalent to .get(key).is_some(). It's impossible to get a None when .contains_keys returns true.

@whfuyn whfuyn changed the title vm: remove redundant state existence check Remove redundant state existence check when loading inputs/outputs Apr 18, 2024
dr-orlovsky
dr-orlovsky requested changes Apr 18, 2024
View reviewed changes
Copy link
Member

@dr-orlovsky dr-orlovsky left a comment

Choose a reason for hiding this comment

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

Thank you! You actually has spotted a bug in the code: the procedure should fail if the state is absent.

@dr-orlovsky dr-orlovsky self-assigned this Apr 18, 2024
@dr-orlovsky dr-orlovsky added the bug Something isn't working label Apr 18, 2024
@dr-orlovsky dr-orlovsky added this to the v0.11.0 milestone Apr 18, 2024
@whfuyn whfuyn force-pushed the remove-redundant-state-check branch from 67a3380 to 8dad03f Compare April 19, 2024 05:52
@whfuyn
Copy link
Contributor Author

whfuyn commented Apr 19, 2024

Fixed!

dr-orlovsky
dr-orlovsky approved these changes Apr 19, 2024
View reviewed changes
Copy link
Member

@dr-orlovsky dr-orlovsky left a comment

Choose a reason for hiding this comment

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

ACK 8dad03f

@dr-orlovsky
Copy link
Member

Thank you and congrats on the first contribution to the repo!

@dr-orlovsky dr-orlovsky merged commit e83f07b into RGB-WG:master Apr 19, 2024
18 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dr-orlovsky dr-orlovsky dr-orlovsky approved these changes

Assignees

@dr-orlovsky dr-orlovsky

Labels
bug Something isn't working
Projects
RGB release v0.11
Status: Done
Milestone
v0.11.0
Development

Successfully merging this pull request may close these issues.
2 participants
@whfuyn @dr-orlovsky