search

hdgarrood / purescript-sequences

Various efficient-ish sequence types for PureScript.
http://pursuit.purescript.org/packages/purescript-sequences
MIT License
45 stars 22 forks source link

Purescript 0.9.3 compatibility #24

Closed adamw closed 7 years ago

adamw commented 8 years ago

This is based on the fork by @sardonicpresence (https://github.com/sardonicpresence/purescript-sequences). There are three minor changes:

I know there's another effort under way (PR #23) which, as far as I understand, aims to eliminate unsafePartial with proper Partial constraints propagation. However it seems that the work there stalled, and it would be great to have a working implementation of sequences for psc 0.9.x meanwhile :)

To sum up:

hdgarrood commented 8 years ago

Hi. Is this further along than #23 in any way? My impression was that #23 is pretty much there except that I want Partial constraints to accurately reflect partiality of functions; last time I checked, the tests in #23 did work, I thought.

Also, I'm sorry, but I would rather not merge this until we have Partial constraints in the right places.

adamw commented 8 years ago

Ah, I thought that they fail as the travis build was failing, but now I see that it was a built timeout, so maybe they are passing. I wanted to check this locally as well, but the base branch seems to be deleted, so I'm not even sure how to check out these changes - is there any way to download the patch maybe? Can't find it in the UI ...

hdgarrood commented 8 years ago

Maybe add ".patch" to the URL?

adamw commented 8 years ago

Ah yes that worked :) Even better, I found that you can just checkout the branch using git, even if the original fork is deleted:

  1. git remote add upstream git@github.com:hdgarrood/purescript-sequences.git
  2. git fetch upstream pull/23/head:LOCAL_NAME
hdgarrood commented 8 years ago

Oh cool, good to know!

adamw commented 8 years ago

Ok, checked #23, currently it doesn't even compile due to some Partial constraints not being propagated. I'll try to understand what happens there or just use my fork for now, time allowing :)

hdgarrood commented 8 years ago

Ok cool, thanks for checking :)

mpodlasin commented 7 years ago

Hi. Is there any reason why this pr is not being merged? Anyone? Thanks in advance and have a nice day. :)

hdgarrood commented 7 years ago

Yes, as I said earlier in the thread, it's because neither of the PRs I have received have Partial constraints in the right places. If anyone wants to pick this up I'd happily review, otherwise I will probably get around to doing it myself eventually.

hdgarrood commented 7 years ago

Fixed in #26.