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

Measure upload speed #28

Merged
merged 16 commits into from
Feb 20, 2019
Merged

Measure upload speed #28

merged 16 commits into from
Feb 20, 2019

Conversation

danielhuang
Copy link
Contributor

@danielhuang danielhuang commented Aug 3, 2018

Closes #27

@danielhuang
Copy link
Contributor Author

Upload speed measuring works, however a flag is not yet implemented

@danielhuang
Copy link
Contributor Author

danielhuang commented Aug 3, 2018

Progress display still feels a bit ugly
image
image
image

@danielhuang danielhuang changed the title WIP measure upload speed Measure upload speed Aug 3, 2018
GloriousYellow added 2 commits August 4, 2018 12:02
@jdwall jdwall mentioned this pull request Aug 9, 2018
@mattstrayer
Copy link

Can this get merged in?

cli.js Outdated Show resolved Hide resolved
cli.js Show resolved Hide resolved
cli.js Outdated Show resolved Hide resolved
api.js Outdated Show resolved Hide resolved
cli.js Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Owner

Maybe we should use the arrows to indicate download/upload?

@sindresorhus
Copy link
Owner

Can this get merged in?

It will be merged when it's ready. If you want it merged faster, help out by reviewing and testing this PR ;)

@sindresorhus
Copy link
Owner

@GloriousYellow This is looking great! 🙌

@HaShAkO
Copy link

HaShAkO commented Aug 14, 2018

Hi, is there any plan to include latency as well ? , great job =)

@HavishNetla
Copy link

prevResult is not being set to anything in api.js

@danielhuang
Copy link
Contributor Author

danielhuang commented Nov 1, 2018

I'm thinking of using ⇣⇡ arrows, like in pure. How does that sound?

However, I'm not sure if these arrows should be printed if invoked through a non-tty.

@danielhuang
Copy link
Contributor Author

danielhuang commented Nov 1, 2018

I've implemented a quick prototype:
image
image
I sort of feel like the arrows take away the original "feel" of this tool.

@danielhuang
Copy link
Contributor Author

With the other type of arrows:
image
Maybe this would work better?

@sindresorhus
Copy link
Owner

@GloriousYellow The last one looks good. Let's go with that.

@erasmuswill
Copy link

Any updates on this?

@sindresorhus
Copy link
Owner

@GloriousYellow Still interested in finishing this? :)

@danielhuang
Copy link
Contributor Author

I am not completely satisfied with the naming of --verbose; it doesn't imply measurement of upload speed.

GloriousYellow added 3 commits February 6, 2019 17:09
@danielhuang
Copy link
Contributor Author

image
Should we make the download speed indicator green during the upload test?

GloriousYellow added 2 commits February 6, 2019 17:31
@danielhuang
Copy link
Contributor Author

image

@danielhuang
Copy link
Contributor Author

image
When completed

GloriousYellow added 2 commits February 6, 2019 17:36
@mattstrayer
Copy link

mattstrayer commented Feb 7, 2019

I think it looks better with the blue. The animation on the left indicates enough to the user when a test is running and when it is complete

@sindresorhus
Copy link
Owner

Should we make the download speed indicator green during the upload test?

👍

@sindresorhus sindresorhus merged commit 6eca355 into sindresorhus:master Feb 20, 2019
@sindresorhus
Copy link
Owner

Thank you for your work on this, @GloriousYellow :)

sindresorhus pushed a commit that referenced this pull request Feb 20, 2019
@danielhuang
Copy link
Contributor Author

danielhuang commented Feb 20, 2019

There is a failing test; would that be a concern?

@sindresorhus
Copy link
Owner

Already fixed in a follow-up commit.

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.

Support fast.com's new upload speed test
6 participants