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

Comments on V2 #1

Closed
etorreborre opened this issue Jun 6, 2018 · 1 comment
Closed

Comments on V2 #1

etorreborre opened this issue Jun 6, 2018 · 1 comment

Comments

@etorreborre
Copy link

Not an issue, just a bunch of comments :-).

First of all, this all looks great! Here is a list of questions and comments that I think we need to address in no particular order

  • we need some doc to explain all the concepts at play

  • I like the fact that the effect type is abstracted, however having to type Identity repeatedly becomes annoying fast. I think that typeclass aliases don't work but I would like to have a way to trim that declaration for gens

  • I would like to change the naming, especially in name that come up often like DefaultRecipeDeps which could be abbreviated to Deps since we already are in the DefaultRecipe typeclass

  • I don't understand yet the interplay between book and state

  • I think we need a combinator to create a const recipe where we force a value regardless of the dependencies

  • I wonder if this scales to at least 20-30 recipes which is what we need for our apps. I hope so. I was truly disappointed to see that type-level sets don't scale up

  • I wonder if the Nub constraint is really necessary. Same thing with UniqueMember. Removing them might reduce compilation times

  • I suspect that the machinery in Data.Diverse.Many is enough to let us describe a record containing some existing recipes and override one of them based on the target type but I would like to see what the syntax look like and if we ever need type annotations

  • One thing I like with the current module system we have for generators is that we can "add" several generation constraints using the State monad:

genInboundModelWithOneSimple = 
  genNewArticles >>
  genOneInboundArticleSimple >> 
  genOneInboundArticleConfig

I would like to see such an example encoded with your proposal

  • I would like to try it out on one of our applications like crocodile or ferret because that's the ultimate test :-)

  • how do you want to work on this? Do you want to lead the effort of making the library production-ready and migrating our components or do you prefer me to run wild with it?

@reactormonk
Copy link
Owner

reactormonk commented Jun 6, 2018

we need some doc to explain all the concepts at play

Working on it, already got something half-backed at play. #3

I like the fact that the effect type is abstracted, however having to type Identity repeatedly becomes annoying fast. I think that typeclass aliases don't work but I would like to have a way to trim that declaration for gens

#2

I would like to change the naming, especially in name that come up often like DefaultRecipeDeps which could be abbreviated to Deps since we already are in the DefaultRecipe typeclass

Associated types aren't namespaced, so having a prefix isn't that bad of an idea. It's the same as a toplevel type family.

I don't understand yet the interplay between book and state

Working on documentation. #3

I think we need a combinator to create a const recipe where we force a value regardless of the dependencies

I think this is what you meant: https://github.com/reactormonk/modules/blob/master/src/V2.hs#L18 - #3

I wonder if this scales to at least 20-30 recipes which is what we need for our apps. I hope so. I was truly disappointed to see that type-level sets don't scale up

#4

I wonder if the Nub constraint is really necessary. Same thing with UniqueMember. Removing them might reduce compilation times

Nub should actually reduce compile-times, because it's once-only. I could replace UniqueMember with something that picks the first one found, which should speed things up. Also added to #4

I suspect that the machinery in Data.Diverse.Many is enough to let us describe a record containing some existing recipes and override one of them based on the target type but I would like to see what the syntax look like and if we ever need type annotations

There's an example in https://github.com/reactormonk/modules/blob/master/src/HedgehogExample.hs#L56 - the @m needs to be annotated :-( - maybe I can help out the compiler somehow here. #5

One thing I like with the current module system we have for generators is that we can "add" several generation constraints using the State monad ...

Yeah, put them into the book part. See #3 - replacing recipes.

I would like to try it out on one of our applications like crocodile or ferret because that's the ultimate test :-)

Lemme complete at least #3 beforehand.

how do you want to work on this? Do you want to lead the effort of making the library production-ready and migrating our components or do you prefer me to run wild with it?

Poke it as much as you can - I'll write up documentation on the way to Zurihac. Maybe I can recruit a few people there.

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

No branches or pull requests

2 participants