-
Notifications
You must be signed in to change notification settings - Fork 21
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
Update Polysemy.Random to use the newer System.Random "Uniform" and "UniformRange" interfaces. #81
base: master
Are you sure you want to change the base?
Conversation
Note that there are a few other ways these changes could have been handled:
|
In the variant of |
Interesting... At first I didn't like it but I'm opening to the idea. Where's the variant you use?
Anyway, I think it's a bigger change than I'm proposing in this PR. If other people are enthusiastic about it I can participate in the work, starting with opening a new Issue... |
It's pretty banal: https://github.com/tek/polysemy-hasql/blob/main/packages/db/lib/Polysemy/Db/Interpreter/Random.hs anyway, I just wanted to ask your opinion, you can proceed with this without changing the effect further. I haven't used it enough to have an informed view on the matter. so, PR looks fine to me! |
Thinking about it more, I don't think this suggestion is the way forward.
I think most people looking for a |
It's awkward using the existing Polysemy.Random, because it maps to the
Random
class, which the current documentation suggests shouldn't be used in new code.This pull request fixes that, and adds relevant unit tests and QOL details.