feat(config): Add custom config for init private flag #4377
Conversation
|
I do not understand this change fully, can you explain your aim a bit more? Are you trying to make private-flag the default? Do we have to cast this to a string? Can we just do |
src/cli/commands/init.js
Outdated
| @@ -31,6 +31,8 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg | |||
| email: config.getOption('init-author-email'), | |||
| url: config.getOption('init-author-url'), | |||
| }; | |||
| const _defaultPrivateFlag = String(config.getOption('init-private')); | |||
| const defaultPrivateFlag = _defaultPrivateFlag === 'undefined' ? '' : _defaultPrivateFlag; | |||
There was a problem hiding this comment.
Why not simply
const defaultPrivateFlag = config.getOption('init-private') || '';
This is actually safer since it allows the literal 'undefined' value too. (not that I expect it to be used but it is "more accurate")
There was a problem hiding this comment.
Hi. I tried this way first. But this not pass the lint test. :(
There was a problem hiding this comment.
You can see here: https://circleci.com/gh/yarnpkg/yarn/tree/pull%2F4377
Ok, I just thought of a shorter way. Let me try.
|
@BYK about this, |
|
In this PR, I would like to add the ability for users to use |
src/cli/commands/init.js
Outdated
| @@ -94,7 +94,7 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg | |||
| { | |||
| key: 'private', | |||
| question: 'private', | |||
| default: '', | |||
| default: String(config.getOption('init-private') || ''), | |||
There was a problem hiding this comment.
This's a string value, and L157 will pass string value of private flag to boolean
There was a problem hiding this comment.
Yarn automatically detects "magic" boolean strings true and false and converts the value into booleans. Try and see yourself ;)
|
Hey, sorry for the very late turnaround :( I tried adding |
|
Hey, I tried your trick with my |
|
It did work for me. What do you see when you run |
|
@BYK here are my screen and
|
I've tested this locally and it works but apparently, the lockfile parser is allergic to Windows-style line ending so it may be that if you edited the file manually. Make sure you have Unix-style line endings and it should work. |
|
Added a PR for |
|
But, what about enable setting default config like another init option :'( ? |
Good point. Added a comment. LMK if that's not the case. Also shouldn't we have at least one test case for this? |
|
@pierreneter let's finish this with a test case! 😉 |
BYK
dismissed
their stale review
Removing myself from reviewers since I edited/wrote code for the PR
| @@ -96,7 +96,7 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg | |||
| { | |||
| key: 'private', | |||
| question: 'private', | |||
| default: '', | |||
| default: config.getOption('init-private') || '', | |||
There was a problem hiding this comment.
any reason this shouldn't default to false?
There was a problem hiding this comment.
Hm, because, default this's not set yet. It will not add to package.json. If set it's false, always it will be add to package.json: private: false.
BYK
changed the title
**Summary** Here is a small custom; I add this because it relates to https://yarnpkg.com/en/docs/cli/init#toc-setting-defaults-for-yarn-init. I discovered this was necessary while writing introduces for this flag. **Test plan** New init and config tests.
Summary
Here is a small custom; I add this because it relates to https://yarnpkg.com/en/docs/cli/init#toc-setting-defaults-for-yarn-init. I discovered this was necessary while writing introduces for this flag.
Test plan
New init and config tests.