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

fixed issue where latency table was never rendered #26

Merged
merged 3 commits into from
Jun 26, 2016

Conversation

GlenTiki
Copy link
Collaborator

I updated some docs, too :)

-i/--input FILE The body of the request. default: `undefined`.
-H/--headers K=V The request headers. default: `undefined`.
-B/--bailout NUM The number of failures before initiating a bailout. default: `undefined`.
-p/--progress Display the progress bar. default: `true`.
Copy link
Owner

@mcollina mcollina Jun 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this switch. It would be --no-progress in case, but why would this be needed? If it's not a TTY it's omitted anyway.

Copy link
Collaborator Author

@GlenTiki GlenTiki Jun 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the fiasco with the npm progress bar, some people might like this option? I just thought it might be nice to give users the option. Also, I chose --progress over --no-progress to match npm, as well.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we should document --no-progress

@mcollina
Copy link
Owner

Also, it conflicts with #25 :(. Can you please update this one so that it applies cleanly?

@GlenTiki
Copy link
Collaborator Author

Fixed nits and changed progress to no-progress 👍

version: 'v',
help: 'h'
},
default: {
connections: 10,
pipelining: 1,
duration: 10,
progress: true,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why progress?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woops, messed up the defaults stuff. will fix

@mcollina mcollina merged commit e4003f4 into mcollina:master Jun 26, 2016
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.

2 participants