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

Fix Issue 1004: effect order in filterAMissing #1005

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

j6carey
Copy link
Contributor

@j6carey j6carey commented May 28, 2024

The order of Applicative effects in filterAMissing
was incorrect, causing the order of effects to
differ from key order and be influenced by how
the binary tree was balanced.

The fix is to arrange that effects arising from
the key and value at an internal node come after
those in its left branch instead of before.
(Regardless of this fix such effects come before
those effects arising from the right branch.)

This change also expands test coverage
to detect a regression of this fix.

The order of Applicative effects in filterAMissing
was incorrect, causing the order of effects to
differ from key order and be influenced by how
the binary tree was balanced.

The fix is to arrange that effects arising from
the key and value at an internal node come after
those in its left branch instead of before.
(Regardless of this fix such effects come before
those effects arising from the right branch.)

This change also expands test coverage
to detect a regression of this fix.
@j6carey
Copy link
Contributor Author

j6carey commented Jun 5, 2024

Since I lack permission to initiate a continuous integration workflow or assign a reviewer, would someone be so kind as to trigger those steps?

@treeowl
Copy link
Contributor

treeowl commented Jun 5, 2024

Sorry I missed this.

@treeowl treeowl merged commit 5d4bc2e into haskell:master Jun 5, 2024
10 checks passed
@treeowl
Copy link
Contributor

treeowl commented Jun 5, 2024

Thanks!

@j6carey j6carey deleted the issue-1004-filterAMissing branch June 5, 2024 21:42
@j6carey
Copy link
Contributor Author

j6carey commented Jun 5, 2024

Thanks for the CI and merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants