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

First Pass At an FSharp Koan for Type Providers Resolving #77 #79

Open
wants to merge 36 commits into
base: fsharp
Choose a base branch
from

Conversation

normanhh3
Copy link
Contributor

This is the official PR for review.

@ChrisMarinos
Copy link
Owner

I've always thought that a koan for type providers would be nice, but I thought it would be hard to come up with a clean way to fit the format. However, I really like the direction this is headed in, and I think it could be a clean way to integrate type providers with the koans. I have a few suggestions if you're interested in doing a little extra work on this.

  1. I think it it's important to really emphasize the way that type providers provide design-time feedback. I think it would be nice to have a koan that forces the user to enter or edit a CSV column name in order to see what the intellisense shows. I could also see a "TRY IT" comment asking them to try changing the value in the csv file to see what happens.

  2. In addition to the link to the type provider docs, it'd be nice to add some text or possibly even a koan to hint at the other types of other type providers that are available without requiring the user to click-through.

  3. It's a small thing, but what do you think about changing the contents of the csv to a different data set besides language popularity? I like the idea of a csv, but I think a slightly larger data set might encourage the user engage a bit more. Maybe something sort of like the stock example with 3-4 columns and 10-15 lines?

Either way, thanks so much for this contribution. It looks like a great idea!

@normanhh3
Copy link
Contributor Author

normanhh3 commented Oct 6, 2018

I agree with all of your points.

  1. This should be really easy to implement.

  2. Agreed. How about the first pass is some additional links to the more popular type providers, and then I will submit additional PR's to work out a couple more providers? How do you feel about embedding a small database so the SqlClient provider could be tested out? That one seemed to be too much overhead, but I am interested in your thoughts.

  3. Yeah, I thought about stock data too, went to go find some from the CSV type provider reference and the link to Google and couldn't get any content! Otherwise I would have gone with that. Any problem with just using the same CSV data in the other Koan?

  4. Should this Koan be ordered differently?

Let me see what I can cook up.

@ChrisMarinos
Copy link
Owner

For 2, I think just a comment referencing the fact that there are additional types of type providers is sufficient. I agree that putting in a full SqlClient is a bit too much overhead. Just something to get the user thinking about other options sounds good to me.

For 3, I think it's fine to use the same data as the other koan. You could even remix the prompt from the problem to have the user solve a problem with the data using a type provider. If you do that, I'd start with a couple easy "fill in the blank" koans leading up to the open-ended problem.

For 4, I would put this koan last, especially if you go with the idea of using a more open-ended prompt above.

@normanhh3
Copy link
Contributor Author

Ugh... Looks like I wound up committing a bunch of items I didn't intend to commit here. :-/ Let me go clean that up.

@normanhh3
Copy link
Contributor Author

@ChrisMarinos ok, this looks like it is now actually ready to be reviewed again - I think.

Apologies for the messy interrupts in there. Hopefully squashing commits will take out the messy stops/starts for the final merge into the main branch?

@ChrisMarinos
Copy link
Owner

Thanks for the update to this! I’ll have a look at it when I return from holiday next week.

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.

2 participants