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

Resolve config at runtime rather than compile time #22

Merged
merged 1 commit into from
Feb 29, 2016
Merged

Resolve config at runtime rather than compile time #22

merged 1 commit into from
Feb 29, 2016

Conversation

lpil
Copy link
Contributor

@lpil lpil commented Feb 28, 2016

Use of module attributes means that these values are resolved at compile
time, and thus do not change when projects using this lib change their
mix config. Unless they wipe the library and manually compile them that
is.

I've moved them to be resolved at compile time, which fixes this
problem. Mix conf is stored in an ETS table, so it's still v fast :)

Use of module attributes means that these values are resolved at compile
time, and thus do not change when projects using this lib change their
mix config. Unless they wipe the library and manually compile them that
is.

I've moved them to be resolved at compile time, which fixes this
problem. Mix conf is stored in an ETS table, so it's still v fast :)
@luc-tielen
Copy link
Contributor

Looks great, thanks! 😃
I think I overlooked this because I usually keep the iterations fixed when I'm working on a project (100 - 1000 for quick tests, 10000 for CI)..
Now we just have to wait till parroty merges this in..

parroty added a commit that referenced this pull request Feb 29, 2016
Resolve config at runtime rather than compile time
@parroty parroty merged commit 6572064 into parroty:master Feb 29, 2016
@parroty
Copy link
Owner

parroty commented Feb 29, 2016

Thanks!

@parroty
Copy link
Owner

parroty commented Feb 29, 2016

I'm not with my laptop and will be releasing package later.

@lpil lpil deleted the fix/runtime-config branch February 29, 2016 14:36
@lpil
Copy link
Contributor Author

lpil commented Feb 29, 2016

Great, thanks!

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.

3 participants