WIP: records with RowToList#6
WIP: records with RowToList#6CarstenKoenig wants to merge 16 commits intopurescript-contrib:masterfrom
Conversation
|
You probably want to use npm-check-updates and run |
|
@justinwoo you mean the Travis CI tooling? I don't have much experience with this but I'll try to be honest: I expected something like this - that's why I WIPed |
|
is it ok if I change the .travis.yml to something like https://github.com/justinwoo/purescript-simple-json/blob/master/.travis.yml ? |
|
sorry for the stupid commits/patches - sometimes I miss the most obvious things would love to hear opinions on this |
|
this is supposed to work towards "fixing" #7 |
test/Main.purs
Outdated
| show a = genericShow a | ||
| instance encodeJsonLiteralStringExample :: EncodeJson LiteralStringExample where | ||
| encodeJson a = encodeLiteralSumWithTransform id a | ||
| encodeJson a = encodeLiteralSumWithTransform (\x->x) a |
There was a problem hiding this comment.
Why replace id with a hand-written id?
There was a problem hiding this comment.
ah yes - thanks for reminding me, I wanted to check that out
for some reason id was not in scope anymore and instead of trying to add yet another yak on my shaving stack I opted to just put it in myself
I'll try to check this out tomorrow
There was a problem hiding this comment.
ah I see - that's identity from Control.Category right?
There was a problem hiding this comment.
yeah ide-server figured that out for me ;)
I understand the reasoning but it's really hard to find using pursuit when you look for the usual a -> a
There was a problem hiding this comment.
that's why there's typed hole search: https://github.com/paf31/24-days-of-purescript-2016/blob/master/23.markdown
There was a problem hiding this comment.
fair enough I guess (I'm probably not using that feature enough)
|
Hey @CarstenKoenig is there any progress on this or have you abandoned it? |
|
Hi @fsoikin - not at all - I guess I should have a look and see if the dependencies are all updated by now (they probably are) - the code itself should work and I would love some comments on it. I'm at a conference right now but I'll try to give a in the next few days. |
|
I updated the dependencies - hopefully it'll now run the test on travis successfully too |
|
ok seems fine - what do you think? If it's ok with you I'll gonna rebase it, close this one and submit a new/clean PR |
fsoikin
left a comment
fsoikin
left a comment
There was a problem hiding this comment.
This looks ok to me.
I wouldn't mind some comments on classes and instances, but overall fine.
| -- | A function for decoding `Generic` sum types using string literal representations | ||
| decodeLiteralSum :: forall a r. Rep.Generic a r => DecodeLiteral r => Json -> Either String a | ||
| decodeLiteralSum = decodeLiteralSumWithTransform id | ||
| decodeLiteralSum = decodeLiteralSumWithTransform (\a -> a) |
There was a problem hiding this comment.
ah yes ... it took me some time to find it and my tooling was broken ... I'll fix it
|
concerning the documentation: I'm not sure what style you want here - especially for the instances - do you want some kind of high-level explanation on how the parts fit together? |
|
I opened a clean version of this here: #8 - hope this helps you |
|
Regarding documentation - I meant some light description of what the classes and their variables mean. For example, the meaning of This code is dense and abstract enough that it takes a significant effort to get through it, and some explanation of how to read it would be helpful. Not a hard demand by any means. I'm not even a stakeholder in this repo, just a consumer. |
|
@CarstenKoenig Closing this since #8 is merged; let me know if there's stuff here you need me to re-open. |
|
no it's all right - everything here was in #8 - thanks again for merging |
5 participants
Hi,
this is still work in progress - but is this something you would appreciate?
Basically this should be the first step to move this forward to PureScript 0.12.0
I removed the
purescript-genericsparts which are no longer suported AFAIK and usedRowToListto get records working again using techniques borrowed/inspired from here: https://github.com/justinwoo/purescript-simple-jsonFor the tests I just upgraded to
Effectand all are green and look fine to me (but please take a look)What is probably not ready to be merged is the version bounds for the
bower.json(I might have messed up some version numbers and maybe I missed one all together)I developed/tested this using psc-package with my own package-set (in which I just included
argonaut-coreandargonaut-codecs) - so that probably have to go too.