search

aristidb / aws

Amazon Web Services for Haskell
BSD 3-Clause "New" or "Revised" License
238 stars 107 forks source link

Add DynVal instances for V.Vector and M.Map #192

Closed angerman closed 8 years ago

angerman commented 8 years ago

I think we are missing the List and Map DynVal instances. This is my stab at it. Feedback welcome! What else is missing the DMap and DList changes?

angerman commented 8 years ago

So this patch fails due to traverse not being in ghc 7. What should we do about that?

aristidb commented 8 years ago

Of course traverse is available in ghc 7, it's just in Data.Traversable :)

Can you adjust the pull request to just import that module?

angerman commented 8 years ago

I've just asked on #haskell this today. And was advised to use base-compat? What do you think about that approach?

aristidb commented 8 years ago

I don't understand what's wrong with importing Data.Traversable that would require using base-compat?

aristidb commented 8 years ago

Discussion about the difference between DynVal instances for [] and V.Vector:

12:52:09 <angerman> can you elaborate?
12:52:20 <aristid> oh wait, this is a little misleading i guess, the V.Vector and [] instances are meant to do something completely different?
12:52:25 <aristid> just look at the [] instance
12:52:28 <aristid> way above yours
12:52:37 <aristid> well, time for lunch :D bbl
12:53:34 <angerman> Yep. Initially I tried to get it work with lists, but there's the set lifting, 
12:55:15 <angerman> But the choice for map and vector are actually, due to the introduction in #177.
12:55:16 <angerman> https://github.com/aristidb/aws/pull/177/files
12:55:28 <angerman> this should be somewhat symmetric.
13:34:18 <angerman> let me know when you are back.
13:52:01 <aristid> back
13:53:43 <aristid> hmmm
13:53:57 <aristid> i feel a bit uneasy about this, but then i'm not as familiar with this code as ozataman
13:54:21 <aristid> can you add a comment to the list and Vector instances that and how they differ?
13:56:10 <aristid> i think i'd merge it then, even if i still feel a little queasy about the whole dichotomy

CC @ozataman