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

Add more Type Class deriving details #338

Merged
merged 12 commits into from
Nov 5, 2020

Conversation

milesfrain
Copy link
Contributor

@milesfrain milesfrain commented Jul 21, 2020

From today's conversation on Slack.

Fixes #155

I think there are better ways to organize this content, but wanted to track this first version.

@milesfrain milesfrain marked this pull request as ready for review July 21, 2020 02:57
@Thimoteus
Copy link
Contributor

Would this be an appropriate place to put info about eta abstraction to help fix some stack overflow issues?

@milesfrain
Copy link
Contributor Author

Yeah. That seems like a good addition.
Could include something similar to: https://github.com/purescript-contrib/purescript-argonaut-codecs/blob/master/README.md#deriving-instances
Might also be good to link to https://github.com/purescript-contrib/purescript-argonaut-generic for an example of writing generic support for your own classes.

@milesfrain
Copy link
Contributor Author

This should be ready to merge now (pending any additional feedback).

language/Type-Classes.md Outdated Show resolved Hide resolved
language/Type-Classes.md Outdated Show resolved Hide resolved
language/Type-Classes.md Outdated Show resolved Hide resolved
language/Type-Classes.md Outdated Show resolved Hide resolved
language/Type-Classes.md Outdated Show resolved Hide resolved
language/Type-Classes.md Outdated Show resolved Hide resolved
Points suggests a collection of {x,y} values.
@milesfrain
Copy link
Contributor Author

Applied all of @thomashoneyman's suggestions

Copy link
Member

@thomashoneyman thomashoneyman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple more small suggestions for the content but this otherwise is looking good to me. With those updated I think this is good to go.

language/Type-Classes.md Outdated Show resolved Hide resolved
language/Type-Classes.md Outdated Show resolved Hide resolved
language/Type-Classes.md Outdated Show resolved Hide resolved
Copy link
Member

@thomashoneyman thomashoneyman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice addition!

@hdgarrood
Copy link
Collaborator

This is good, but the stuff beyond Generic is not suitable for the language reference, as it's more of a how-to/explanation and covers things which aren't part of the language.

@milesfrain
Copy link
Contributor Author

milesfrain commented Sep 6, 2020

Section removed. Hoping to include that in some sort of language "guide" in the future.
@Thimoteus, do you have strong feelings on whether that material makes it into these docs? I added it per your suggestion.

@hdgarrood
Copy link
Collaborator

Thanks. I think it would be good to put those other paragraphs into the “guides” directory instead. Even if there’s a bit of overlap between the reference and the guide, I think that’s fine.

@hdgarrood
Copy link
Collaborator

Oh sorry; to clarify, I think the stuff under the “Deriving with Generic” heading is also not really suitable for the language reference.

@thomashoneyman
Copy link
Member

I agree the content deserves a place in the documentation (rather than being removed altogether) and perhaps we could link to it here as further reading in the guides section.

Copy link
Member

@thomashoneyman thomashoneyman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with this content ending up in the guides section and linked from the language reference.

@milesfrain
Copy link
Contributor Author

The first two sections (built-in and newtype) are duplicated across both the guide and the language reference. I feel like this content is equally relevant for the guide, since the generic section builds on these. There are certainly other ways to organize this material, such as:

  1. Relocating just the Generic instructions to generics-rep as proposed in https://github.com/purescript/purescript-generics-rep/issues/41
  2. Linking back to the prerequisite sections of the language reference
  3. Deleting the deriving guide in this docs repo

But I'm thinking it's best to save this reorganization work for later.

My understanding is that the language folder is supposed to be as concise as possible. Is this content considered the language "specification", "reference", or both? Some languages use those terms interchangeably https://golang.org/ref/spec

I'm still unsure what's considered within scope of these sections. In hindsight, it seems like the Generic example merged in #331 was scope creep.

@hdgarrood
Copy link
Collaborator

The duplication is not a problem in my mind. I’d much rather have a little bit of duplication here and there than have documentation resources which try to cater to everyone at once.

The language/ directory is more of a reference than a specification. In my mind a specification ought to tell you how everything behaves in a much more precise manner, and is aimed more at implementers (whereas a reference is aimed at users). It would be good to have a specification one day (which would be another separate resource), but realistically that’s unlikely to happen any time soon.

I agree that the example merged in #331 probably didn’t belong in the language reference either, in retrospect. The main criterion is whether something is part of the language or whether it is part of a library; if it’s the latter, it probably shouldn’t go in here. Of course the boundaries aren’t always clear: it’s difficult to discuss do notation without mentioning monads, for example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document derive newtype syntax and capability
5 participants