Should we allow optional arguments or not? about node-zendesk HOT 5 CLOSED

I've taken the form as shown here https://github.com/blakmatrix/node-zendesk/blob/master/lib/client/attachments.js#L15
a bit more complex is https://github.com/blakmatrix/node-zendesk/blob/master/lib/client/useridentities.js#L31 - except with that one the format is the optional bit

When I worked at nodejitsu back in the day I used the form of this (https://github.com/nodejitsu/jitsu/blob/master/lib/jitsu/commands/env.js#L37-L46) for optionals note: we used the flatiron framework, utile was a utility library and simply utilize these mechanisms https://github.com/flatiron/utile/blob/master/lib/args.js

I personally prefer simplicity, utilization of an options object that can be passed allong as {} or {locale: 'fr'} but is explicit in requirement.

from node-zendesk.

Also regarding simplicity, for GET /api/v2/help_center/{locale}/sections/{id}.json We could also create a method named createBySectionWithLocale but I feel as if that is almost overly verbose and I'd personally use the option format over that.

from node-zendesk.

Using an options object would break the backward compatibility.

Since in both end points we need to use sectionID to specify where the article belongs to, the prepositional phrase bySection could be omitted. I finally choose createWithLocale for this new end point.

I am also thinking we can apply the same rule to name an end point: Methods like listByUser is where user need to pass userID to identify results, so they use preposition By. While WithLocale means the result is not only identified by Locale but also requires other information like sectionID in this example.

from node-zendesk.

Since the locale is mandatory when creating something new in the help center, I'm of the opinion that we just utilize the option object. Seems straightforward and included already in the post body.

Plus the documentation reads:

You must specify an article title and locale. The locale can be omitted if it's specified in the URL.

@nnabuuu if this is the case and we choose to go this route, then we should disregard the create endpoints that have the locale defined and only use ones like POST /api/v2/help_center/sections/{id}/articles.json.

Meanwhile, listing and updating should go unchanged and still have the byLocale methods.

EDIT: Saw that you made that PR to update the help center endpoints. I'm cool with the changes since it's been done, but it would be nice to just have a simple set of CRUD methods instead of all the variance.

from node-zendesk.

OK, let's do this: I will make 2 methods byLocale and withLocale, but I would add a warning message in byLocale, we wouldn't delete the byLocale method until a major version upgrade.

from node-zendesk.