Skip to content
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

New tutorial for argonaut-codecs #62

Merged
merged 4 commits into from
Sep 8, 2019
Merged

Conversation

thomashoneyman
Copy link
Contributor

@thomashoneyman thomashoneyman commented Sep 4, 2019

What does this pull request do?

Closes #48 and adds an in-depth tutorial to introduce new users to the library, provide them with the tools they need to perform common tasks, and includes a few problem situations along with their solutions at the end. I think it's critical that a new Argonaut user feels comfortable creating encoders and decoders for simple data types without having to look at tests, search for random examples across the web, or rely on Haskell or Scala documentation instead.

Where should the reviewer start?

All changes are to the README.

How should this be manually tested?

There is no new code to test, but if you'd like to try things out in the REPL and make sure there aren't errors, that would help; this also deserves an error-checking proofread.

To read the pretty, non-diff version of the file, use this link:
https://github.com/purescript-contrib/purescript-argonaut-codecs/blob/05d25c2ec89380dc7c6e8fdbc755150983bc0115/README.md

Other Notes:

If I am missing any common scenarios that really ought to be covered by a tutorial, I am happy to add them in as well and am open to suggestions to improve this! They can also be subsequent pull requests.

@thomashoneyman thomashoneyman self-assigned this Sep 4, 2019
@thomashoneyman thomashoneyman added the type: documentation Improvements or additions to documentation. label Sep 4, 2019
@thomashoneyman thomashoneyman requested a review from garyb September 4, 2019 01:38
@paldepind
Copy link

As someone who's been confused about Argonaut and about how I was supposed to do things, this tutorial is awesome! It answers pretty much all my questions and after reading I feel like I have a good grasp on things.

Here's some minor ideas on how to make things even better:

  • This seems to be a general introduction to Argonaut so maybe it should actually be in the purescript-argonaut-core repository? Perhaps that is where new users will look first and expect to find beginner documentation? At the very least, the overview over the Argonaut ecosystem in the Documentation system feels like it belongs in the core repo.

  • It would be useful if, the first time a type or function is mentioned, there is a link to it's Pursuit documentation.

  • To me, the very start of the tutorial seemed to repeat some things also mentioned earlier. For instance, it start with "Argonaut is a library for working with JSON in PureScript.". I don't think it's necessary to mention that at this point. Then the tutorial mentions that Json is defined in the core which is also mentioned earlier in the documentation section.

Anyway, this tutorial is great. Thanks a lot for writing it.

@thomashoneyman
Copy link
Contributor Author

Thanks for the feedback!

This seems to be a general introduction to Argonaut so maybe it should actually be in the purescript-argonaut-core repository? Perhaps that is where new users will look first and expect to find beginner documentation? At the very least, the overview over the Argonaut ecosystem in the Documentation system feels like it belongs in the core repo.

Good points!

It's a bit confusing having three separate libraries which can be used separately but which are designed to be used together. In each case a possible newcomer ought to get a feel for what Argonaut is about, even if they aren't looking at the -core repository. And so I'm of the opinion that all of the Argonaut libraries ought to include a short snippet about Argonaut, including the overview of the Argonaut ecosystem. Maybe that's overkill; regardless, I'm thinking to open PRs for each of the repositories including the main argonaut repo.

With regards to being a general introduction to Argonaut: that's true. The -core repo focuses on the core Json type and the basic functions for operating on it, as people can import that library and use it without this one (for example, purescript-affjax and others use the Json type from core only). This repo focuses on a more common application use case, the EncodeJson and DecodeJson type classes.

I don't feel like the topics talked about here (encoding and decoding Json) are appropriate for the -core repo for that reason; it doesn't touch encoding / decoding.

It would be useful if, the first time a type or function is mentioned, there is a link to it's Pursuit documentation.

This is a good point. I'll add those links.

To me, the very start of the tutorial seemed to repeat some things also mentioned earlier. For instance, it start with "Argonaut is a library for working with JSON in PureScript.". I don't think it's necessary to mention that at this point. Then the tutorial mentions that Json is defined in the core which is also mentioned earlier in the documentation section.

Yes, there are some redundancies there. I think the tutorial would be better with those removed as well. Thanks!

address feedback from @paldepind and improve relevant sections of the tutorial
@paldepind
Copy link

And so I'm of the opinion that all of the Argonaut libraries ought to include a short snippet about Argonaut, including the overview of the Argonaut ecosystem. Maybe that's overkill; regardless, I'm thinking to open PRs for each of the repositories including the main argonaut repo.

That sounds like a great idea. And each of these could have a "beginners see this tutorial" link.

The changes looks great 👍

@thomashoneyman
Copy link
Contributor Author

Excellent! I'll let this sit for another day or two, but if there are no further comments then I will merge this in. Further updates can always be made.

@thomashoneyman thomashoneyman merged commit 699073d into master Sep 8, 2019
@thomashoneyman thomashoneyman deleted the thomashoneyman-docs branch September 8, 2019 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation Improvements or additions to documentation.
Development

Successfully merging this pull request may close these issues.

Add example to readme of automatic decoding into simple record types
2 participants