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

Fix #884 #891

Merged
merged 2 commits into from Jun 26, 2017
Merged

Fix #884 #891

merged 2 commits into from Jun 26, 2017

Conversation

angadgill92
Copy link
Contributor

Fix for #884
in lib/usage.js : line 416

  • Replace JSON.stringify(value) with '"' + value + '"' when typeof value is string

@angadgill92
Copy link
Contributor Author

Please review 🙂

@bcoe
Copy link
Member

bcoe commented Jun 5, 2017

@angadgill92 thanks for the contribution, I'm hoping to put some time against yargs this week.

@angadgill92
Copy link
Contributor Author

@bcoe Sure 🙂

@bcoe
Copy link
Member

bcoe commented Jun 10, 2017

@angadgill92 finally sitting down and doing a bit of hacking on yargs; don't suppose I could convince you to write a unit test for this?

@angadgill92
Copy link
Contributor Author

@bcoe Will do 👍 🙂

@bcoe
Copy link
Member

bcoe commented Jun 16, 2017

@angadgill92 will add a test this weekend hopefully and land this, unless you beat me to the punch 😄

@bcoe bcoe merged commit 628be21 into yargs:master Jun 26, 2017
@angadgill92 angadgill92 deleted the 884-fix branch June 29, 2017 18:35
@bcoe
Copy link
Member

bcoe commented Sep 3, 2017

@angadgill92 this is queued up to be released with the next major release of yargs, I would love your help testing since it's looking like yargs@9 is going to have quite a few changes:

npm i yargs@next --save

@angadgill92
Copy link
Contributor Author

@bcoe Hey, sorry I couldn't add tests earlier, I got busy interviewing for jobs. I would really like to help out over weekends though 👍

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.

None yet

2 participants