-
Notifications
You must be signed in to change notification settings - Fork 33
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
WIP Crutches.Option module #43
Conversation
Hmm, weird that the build is failing. Might have to do with other warnings, but I doubt that'd make it segfault. Temporarily fixing all warnings (will drop from commit history if unsuccessful). |
That seems to have done it. If wanted, that commit could go somewhere else. |
""" | ||
@spec combine(list(any), list(any)) :: list(any) | ||
def combine(opts, config) do | ||
Crutches.Option.combine(opts, config, &Elixir.Keyword.merge(&1, &2)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just call combine(args)
instead of Crutches.Option.combine(args)
. Ditto for other occurrences.
P.S. Great work dude!
@KNRZ Incorporated your feedback. Currently working on the last checkbox. |
Crutches.Option module Functions to turn this: ``` @function_name [ valid_options: [:some, :option], default_options: [ some: "foo" option: "bar" ] ] def function_name(args, opts \\ @function_name[:default_options]) def function_name(args, opts) do Crutches.Keyword.validate_keys!(opts, @function_name[:valid_options]) options = Keyword.merge(@function_name[:default_options], opts) ... end ``` Into this: ``` @function_name [ valid: [:some, :option], defaults: [ some: "foo", option: "bar" ] ] def function_name(args, opts \\ @function_name[:defaults]) def function_name(args, opts) do # This validates and merges the options, throwing on error. opts = Crutches.Options.combine!(opts, @function_name) ... end ```
Functions to turn this:
Into this:
For more context, see #42.
Refactoring of the library to make use of these new functions is not going to happen in this PR, in order for everyone to be able to benefit from these functions as quickly as possible.
TODOs:
Option.combine!(options, configuration)
— throwingOption.combine(options, configuration)
— non-throwingOption.combine!(options, configuration, combinator)
— throwing, with custom combinator.Option.combine(options, configuration, combinator)
— non-throwing, with custom combinator.Crutches.Keyword
intoCrutches.Option