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

Fix hypothetical result of condition #940

Merged
merged 2 commits into from Jan 4, 2023
Merged

Fix hypothetical result of condition #940

merged 2 commits into from Jan 4, 2023

Conversation

xsebek
Copy link
Member

@xsebek xsebek commented Jan 1, 2023

When a bind occurred in hypothetical evaluation, then the Value was a VResult that we silently discarded.
Now it is handled and all other values are logged.

@xsebek
Fix hypothetical result of condition
d50996f 
- handle VResult in hypothetical
- log unexpected result values

When a bind occured in hypothetical evaluation then the Value
was a VResult which we used to silently discard. Now it is handled
and all other values are logged.
@xsebek xsebek requested review from kostmo and byorgey January 1, 2023 12:36
kostmo
kostmo approved these changes Jan 1, 2023
View reviewed changes
Copy link
Member

@kostmo kostmo left a comment

Choose a reason for hiding this comment

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

#927 touches this area of code, so I'll be careful to preserve this when I rebase.

byorgey
byorgey approved these changes Jan 1, 2023
View reviewed changes
Copy link
Member

@byorgey byorgey left a comment

Choose a reason for hiding this comment

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

That's it! I knew it must have something to do with VResult...

@xsebek xsebek added the merge me Trigger the merge process of the Pull request. label Jan 4, 2023
@mergify mergify bot merged commit 89d36a4 into main Jan 4, 2023
@mergify mergify bot deleted the fix-hypothetical-result branch January 4, 2023 07:31
This was referenced Jan 4, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kostmo kostmo kostmo approved these changes

@byorgey byorgey byorgey approved these changes

Assignees
No one assigned
Labels
merge me Trigger the merge process of the Pull request.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Counting inventory does not work for goal criteria
3 participants
@xsebek @kostmo @byorgey