sindresorhus / fast-cli

Test your download and upload speed using fast.com
MIT License
2.59k stars 110 forks source link

Add `json` flag #74

Closed jopemachine closed 2 years ago

jopemachine commented 2 years ago

Fixes #50

I replaced the logUpdate API in the previous PR with ink's API. And to avoid showing upload-attributes from the json result when upload is false, added these lines.

I'd appreciate letting me know if there are other tasks to do :)

sindresorhus commented 2 years ago

The --json output cannot be interactive. It should be possible to redirect the output.

sindresorhus commented 2 years ago

It should not include the isDone property.

uploadSpeed should also not be included when not using --upload.

sindresorhus commented 2 years ago

I don't think we should include "downloadUnit": "Mbps". The downloadSpeed and uploadSpeed should always be converted to Mbps if the returned value is not in Mbps.

jopemachine commented 2 years ago

Thanks for your detailed review :) I just updated PR.

sindresorhus commented 2 years ago

I think you missed: https://github.com/sindresorhus/fast-cli/pull/74#issuecomment-1030540745

jopemachine commented 2 years ago

I think you missed: #74 (comment)

I'm sorry for missing your point. I just fixed the PR assuming the convert should be applied when using json option. If I got wrong, I'd appreciate your letting me know.

sindresorhus commented 2 years ago

Thanks :)