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

Enable Bower API options #56

Merged
merged 4 commits into from
Dec 8, 2014
Merged

Enable Bower API options #56

merged 4 commits into from
Dec 8, 2014

Conversation

JorritPosthuma
Copy link
Collaborator

I needed te be able to set the cwd option of the Bower API. This enables that.

@tellnes
Copy link
Collaborator

tellnes commented Dec 4, 2014

Is there any reason not to pass the options argument directly to bower?

It can be done in combination with something like this:

if (typeof options.offline === 'undefined') options.offline = true

Btw, you do not need to create a new pull request when doing modifications. You can just update the branch you are creating the pull request from.

@eugeneware
Copy link
Owner

Yeah, I think passing the options directly might be the way to go.

@sukima
Copy link
Collaborator

sukima commented Dec 4, 2014

I tend to prefer testing undefined / null this way:

if (options.offline == null) options.offline = true;

That catches both null and undefined. You can also simplyfy conditions that depened on false value as

if (options.offline !== false) doSomething

That forces the default regardless of type or value unless false was explicitly assigned to turn it off.

@eugeneware
Copy link
Owner

the typeof xx === 'undefined' is the recommended safe way to test for undefinedness, and is also recommended by the the most excellent tome effective javascript

It's verbose, but nice and safe.

You also want to make sure that the options object is an object as well, so you don't throw an error if the options object is undefined?

@JorritPosthuma
Copy link
Collaborator Author

Better?

@sukima
Copy link
Collaborator

sukima commented Dec 5, 2014

Testing typeof is only needed if your testing the existence of a possibly undeclared variable. Since all objects will return undefined for missing properties it's essentially superfluous.


if (typeof options.bowerOptions === 'object') {
bowerOptions = options.bowerOptions;
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could just be:

bowerOptions = options.bowerOptions || {};

@eugeneware
Copy link
Owner

LGTM 👍

@eugeneware eugeneware merged commit 344aec8 into eugeneware:master Dec 8, 2014
@eugeneware
Copy link
Owner

Thanks @JorritPosthuma for the PR! I've merged and published this as 1.2.0. I've also added you to the contributors list and giving you collaborate access to the repo as per our Open Open Source Contribution Policy.

Cheers!

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.

5 participants