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

Reading the generator from environment? #31

Open
kindaro opened this issue Mar 20, 2021 · 4 comments
Open

Reading the generator from environment? #31

kindaro opened this issue Mar 20, 2021 · 4 comments
Labels
good first issue Good for newcomers

Comments

@kindaro
Copy link

kindaro commented Mar 20, 2021

Currently, the value of the deterministic generator is nailed down in FakerSettings. It is used in the function generateWithSettings.

I propose that this function checks if:

  • A certain environment variable is set.
  • It parses to a number.
  • The value of the generator in the given settings is default.
  • The mode of generation is deterministic.

— And if all are true, the value of the parsed environment variable is used as the generator instead of the default value.

The goal is to make it possible to adjust the generator in a convenient way. Of course there are already ways to adjust the generator, but they all require passing it around the code one way or another, which is rather inconvenient.

This change is conservative and backwards compatible. I can write the code and make a pull request.

@psibi
Copy link
Member

psibi commented Mar 20, 2021

Sounds interesting. What do you think about introducing a new function instead which has the above behaviour you describe:

generateWithEnvSettings :: FakerSettings -> Fake a -> IO a

I'm slightly concerned about the additional IO done for each call to generateWithSettings. What do you think ?

@kindaro
Copy link
Author

kindaro commented Mar 20, 2021

This is a valid concern. I have not measured the cost of access to environment variables — I imagine it to be small but who knows. On the other hand, introducing this new function will result in some code duplication — I cannot see a way to express any one of generateWithSettings and generateWithEnvSettings in terms of the other. Whichever works for you is fine by me!

@psibi
Copy link
Member

psibi commented Mar 20, 2021

On the other hand, introducing this new function will result in some code duplication — I cannot see a way to express any one of generateWithSettings and generateWithEnvSettings in terms of the other.

I guess that's fine. :-)

@kindaro
Copy link
Author

kindaro commented Mar 20, 2021

Alright then, I shall do this within a few days.

@psibi psibi added the good first issue Good for newcomers label Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants