feat(args): improve api dx #488
Conversation
The default platform is set as the current OS. This prevents additional binaries from being downloaded in run-mode apart from being a more sensible default. Arguments to be passed to NW.js instance must be specified after `--` flag (cli) or `options.argv` (js API). --enable-logging flag has been removed as a default argument to NW.js spawn
Fixed some punctuation in the README file.
README.md
Outdated
| version: '0.14.6' | ||
| platforms: ['osx64', 'win32', 'win64', 'linux32', 'linux64'], | ||
| version: 'latest', | ||
| argv: '-- <args>' // see nwjs docs for possible <args> |
There was a problem hiding this comment.
Isn't this supposed to be argv: [/*'<nwjs_arg>', ...*/] (array vs string)
+ Fixed API example - options.argv is an Array. + Defined Separate types for API and CLI for Array/csv
README.md
Outdated
| version: '0.14.6' | ||
| platforms: ['osx64', 'win32', 'win64', 'linux32', 'linux64'], | ||
| version: 'latest', | ||
| argv: '[<nwjs_arg>, ... ]' // see nwjs docs for possible <nwjs_arg> values |
There was a problem hiding this comment.
Still a string, should be argv: ['<nwjs_arg>', ...] (the quotes are around the <nwjs_arg> not the entire array)
README.md
Outdated
| @@ -88,7 +94,7 @@ The path to your node webkit app. It supports [simple-glob](https://github.com/j | |||
|
|
|||
|
|
|||
| #### options.version | |||
| Type: `String` | |||
| Type: `Array``String` | |||
Removed Array as version type and fix quotes in API example.
|
Why don't you ask @adam-lynch to make you a maintainer???? |
With great power comes great responsibility. |
|
ROFL! You are maintaining a lib not saving planet earth.... But @zaygraveyard you have an eye for details... Which is important!!!!! |
I would argue that maintaining a lib can be as important as saving the planet if you recall the left-pad crisis 😝
Thank you 😄 I guess now we wait for @adam-lynch |
|
@CxRes hi there friend, I'm going over the PRs and issues. Could you please comment if this PR is relevant? |
|
I think the PR is rather sensible for the reasons I explained in the first post in this thread as well as #482. @zaygraveyard has also reviewed this and broadly agrees afaik. |
|
@kessler You'll have to give me some time. As it was so long ago, I do not even remember now the changes I had made. In particular, I am not sure about the last of three conflicts. I'll get back to you once I get a chance to have a look again! |
|
@CxRes I don't have to give you anything that is inherently yours! :-) I'll wait patiently. |
|
@kessler Done... You can pull now!!! |
ayushmanchhabra
changed the title
ayushmanchhabra
changed the title
|
@ayushmXn Are you going to be a permanent maintainer in the foreseeable future? |
|
Yes! |
As discussed in #482. Please accept this as well as some of the other very sensible pending PRs.
Breaking Change: The default platform is set as the current OS (using the prexisting OS detection mechanism). This prevents additional binaries from being downloaded when in run-mode apart from being a more sensible default.
Fixed: Arguments to be passed to NW.js instance must be specified after
--flag (cli) oroptions.argv(js API). There were corner cases where this was not happening.Modified:
--enable-loggingflag has been removed as a default argument to NW.js spawn. (User can pass this on their own volition or we can use a--verboseflag to do it for her. Even now,--silentis not linked to--enable-logging.