-
Notifications
You must be signed in to change notification settings - Fork 26
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
Move optional arguments after mandatory arguments #67
Comments
@christianbundy this is "kind of" already doing it in the first line of each function. ...
if(!obj) obj = hmac_key, hmac_key = null
... If the third parameter is Obviously this is not safe at all but 🤷♂️ |
Yep. I think that's overly complicated and makes the API harder to learn / maintain. In my experience, mandatory arguments always come before optional arguments so that you don't have to do stuff like that. |
I fully agree with the change, but it being a breaking change, I'm afraid of cases where dependents just update the version of ssb-keys without updating their code, and then the arguments would be passed, but no error would be emitted, instead the semantics of the arguments are flipped. This may sneakily change the semantics of dependents in a way that's hard to trace. This package being very important for reliability in the SSB ecosystem, I would actually prefer a stable API (even if quirky) over a simple and nice API. |
@staltz I have an idea on how to work on this keeping backward compatibility. I'll publish a PR this weekend so you guys can take a look and decide if the approach is something we want to keep. |
The signature and verification methods have an optional
hmac_key
, which is the second argument, but the third argument is mandatory. Passingnull
everywhere sucks and we should move optional arguments after mandatory arguments.The text was updated successfully, but these errors were encountered: