-
Notifications
You must be signed in to change notification settings - Fork 20
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
setup
API is too loose
#41
Comments
I agree with your sentiment. As far as the String to Atom goes, currently the underlying WpaSupplicant requires an Atom. I know there is a pull request there too, to allow strings to be used. My idea is we should implement our strategy concerning the config, and massage it to confirm with the called APIs. That will also allow us to enforce our contract, and make it more clear to the users. |
in 0.3.x releases, we are now logging an incompatible type for ifnames such as here Next |
as of 0.3.7, you will receive warnings on incompatible settings. later versions will officially deprecate old apis |
in ref to #17, #38 and by extension #40 i would like to start an official discussion of the
setup
api.I don't personally want to support both string and atom keys for things. And this is only one part of this particular issue. I would like to parse and confirm them before passing further down to be messed up anywhere. I think the
parse_settings
function @Hermanverschooten introduced is on the right track, but instead of coxing things into a proper shape, we should enforce the proper shape. I think the public api should raise ArgumentError's when options are not inputted correctly.The text was updated successfully, but these errors were encountered: