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

Allow boolean values in bower.json file created by bower init #1819

Merged
merged 1 commit into from Sep 11, 2015
Merged

Allow boolean values in bower.json file created by bower init #1819

merged 1 commit into from Sep 11, 2015

Conversation

bak
Copy link
Contributor

@bak bak commented Jun 11, 2015

Hi!

While using bower init this morning, I noticed that the resulting bower.json file did not include the "private" key I had chosen a value for. It looks like the behavior of mout.lang.isEmpty() was changed in mout v0.10.0 (discussion), and Bower's dependency on mout introduced this bug, preventing bower init from being able to create a complete bower.json.

Interestingly, this buggy(?) behavior is captured in the bower test for init, where the input includes "private", but it is not in the expected output, so forgive me if this behavior is in fact intended :).

Also, I almost never write JavaScript, so please let me know if there are style/other issues to fix.

Thanks for looking!
<3 Ben

@jisaacks
Copy link
Member

👍

@jisaacks
Copy link
Member

However I think it is worth noting that it looks like with the new implementation of isEmpty, any numbers would also be considered invalid.

@bak
Copy link
Contributor Author

bak commented Jun 16, 2015

Yeah, I wanted to make the most narrow fix I could. Since the bower.json spec documents the expected types (Object, Array, String, Boolean), we should not have to worry about numbers.

@bak
Copy link
Contributor Author

bak commented Jun 16, 2015

I've added a comment to the new method referencing the allowed types and the documentation.

@mlr
Copy link

mlr commented Jul 30, 2015

👍

@bak
Copy link
Contributor Author

bak commented Sep 2, 2015

@sheerun looks like you've been doing a lot of integration work on bower—is there anything I should do to get this merged?

@sheerun
Copy link
Contributor

sheerun commented Sep 7, 2015

I think we should include numbers for forward compatibility

A change in the behavior of `mout.lang.isEmpty()` in Mout v0.10.0
introduced a bug in `bower init`, preventing it from including Boolean
values in "bower.json".

This commit explicitly whitelists the types that are used in the
bower.json spec - Object, Array, String, Boolean - as well as Number for
future compatibility.
@bak
Copy link
Contributor Author

bak commented Sep 11, 2015

Seems reasonable, thanks @sheerun. Pushed a new commit - I've also changed the logic to explicitly whitelist valid values, so this is easier to read and reason about.

@sheerun
Copy link
Contributor

sheerun commented Sep 11, 2015

Looks good! Thanks!

sheerun added a commit that referenced this pull request Sep 11, 2015
Allow boolean values in bower.json file created by `bower init`
@sheerun sheerun merged commit 31b6d59 into bower:master Sep 11, 2015
@bak
Copy link
Contributor Author

bak commented Sep 11, 2015

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants