-
Notifications
You must be signed in to change notification settings - Fork 70
Restore stability for sorting, add tests #197
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
Restore stability for sorting, add tests #197
Conversation
099c9a5
to
84b3e91
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar changes need to be applied to src/Data/Array/ST.js and test/Test/Data/Array/ST.purs.
Could you also add a comment to sort, sortBy and sortWith in Data.Array and Data.Array.ST to document their stability? I think something along the lines of “Sorting is stable: the order of equal elements is preserved.” should be enough.
I updated the compiler version specified in the .github/workflows/ci.yml on master, the CI build should pass after a rebase. |
8efca25
to
23ab0fa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks quite good to me (the build failure is fixed by #198 on master), thank you very much!
CI is failing with this error because I forgot to update
|
CI is now failing with this error. I've submitted purescript/purescript-identity#23 to fix
|
@JordanMartinez we also need to remove the fold1 implementation. Sorry for the churn, @ptrfrncsmrph 😅 |
Finally! Thanks @ptrfrncsmrph! |
Fixes #196