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

Disable progress bar #1962

Merged
merged 5 commits into from
Dec 19, 2016
Merged

Disable progress bar #1962

merged 5 commits into from
Dec 19, 2016

Conversation

olingern
Copy link
Contributor

@olingern olingern commented Nov 21, 2016

Summary
Per #1956, this enables users to specify a JSON key, noProgress, in their package.json to disable the progress bar from showing.

Test plan
Small refactor for testability, created __tests__/cli/index.js file and tests for addition.

Using feature
Adding the following the package.json will disable progress bar output:
"noProgress": true,

@olingern olingern closed this Nov 21, 2016
@olingern olingern reopened this Nov 21, 2016
@torifat
Copy link
Member

torifat commented Nov 21, 2016

IMO, it's better to handle it in yarn config than package.json. Lets see what others think.

@gihrig
Copy link

gihrig commented Nov 21, 2016

it's better to handle it in yarn config than package.json

Config file vs package.json seems to come down to preference.

I would opt for both. The react-boilerplate project makes a point of keeping the project root as clean as practical. That's nice but it adds a lot of complexity to package.json. This choice is enabled by packages that support both options, eslint for example.

@torifat
Copy link
Member

torifat commented Nov 21, 2016

@gihrig I wasn't talking about config file in project folder but global config.

Is there any use case where you want to disable progress only for a specific repo?

@gihrig
Copy link

gihrig commented Nov 22, 2016

@torifat Probly not. I have npm progress bar disabled in global config and never wanted otherwise. So I agree, on second thought, package json makes little sense.

@olingern
Copy link
Contributor Author

@torifat Will refactor this to use yarn config unless there are any objections from others :)

@olingern
Copy link
Contributor Author

@torifat This has been refactored to use yarn config

@torifat
Copy link
Member

torifat commented Dec 18, 2016

LGTM cc @bestander

@bestander bestander merged commit 8f3cb99 into yarnpkg:master Dec 19, 2016
@wclr wclr mentioned this pull request Apr 23, 2017
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.

4 participants