Skip to content

Add encoder for NonEmptyString #98

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

Merged
merged 2 commits into from
Feb 6, 2021

Conversation

d0liver
Copy link
Contributor

@d0liver d0liver commented Feb 4, 2021

Added an EncodeJson instance for NonEmptyStrings and a test for the EncodeJson and DecodeJson instances for NonEmptyStrings.


Checklist:

  • Added the change to the changelog's "Unreleased" section with a link to this PR and your username
  • Linked any existing issues or proposals that this pull request should close
  • Updated or added relevant documentation in the README and/or documentation directory
  • Added a test for the contribution (if applicable)

@thomashoneyman
Copy link
Contributor

Don't worry about the CI failures related to dependencies -- these are due to breaking changes being made in the PureScript 0.14 package set.

@garyb
Copy link
Member

garyb commented Feb 5, 2021

I'd suggest adding a DecodeJson instance for it at the same time, I think all the encode/decodes are paired just now 🙂

@garyb
Copy link
Member

garyb commented Feb 5, 2021

Oh, sorry, I just saw on Slack there already is one!

@thomashoneyman
Copy link
Contributor

@garyb This fixes a slip where we'd only added a DecodeJson instance for NonEmptyString and not included an encoder as well. So no need in this case, though in general you're spot on!

@thomashoneyman
Copy link
Contributor

Ah, you're too fast for me!

@garyb
Copy link
Member

garyb commented Feb 5, 2021

Funny that we both noticed within seconds of each other, 2 hours after I commented 🤣

@d0liver d0liver marked this pull request as ready for review February 5, 2021 05:39
@thomashoneyman thomashoneyman merged commit bbc4d0b into purescript-contrib:main Feb 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants