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

Adding 'progress' config option for monitoring uploads and downloads #96

Merged
merged 1 commit into from Mar 6, 2016

Conversation

barberdt
Copy link

Addressing #82.

Notes
  • This PR does not include any additional test coverage. Curious what your thoughts are there.
  • Added a small update to the upload example code to include this new option.
  • progress vs onProgress vs (any other name). No opinion or preference here.
  • Did not structure the option as an array like transformRequest/Response. There does not seem to be a good default, so no need to merge arrays. However, if consistency is important there, happy to change it.
  • First contribution here, happy to conform to any guidelines I may have missed.

@barberdt
Copy link
Author

The suggestion in #82 also suggests a different method signature that would require parsing the event and passing loaded and total to the configured function. I also have no opinion here. Those params would satisfy my use case, however, sometimes it's nice to have the whole event.

@barberdt
Copy link
Author

@mzabriskie Just wanted to check in on this. If there's anything you'd like me to change/update, please let me know. Happy to discuss other options if this doesn't fit into your roadmap. Thanks!

@Pavel910
Copy link

Pavel910 commented Sep 5, 2015

@mzabriskie @barberdt This would be very useful to me also. Anything we can do to make it to official release?

@willdady
Copy link

@mzabriskie Can this be merged please? @mzabriskie are you still maintaining axios?

@IDontEatSoap
Copy link

+1, please merge.

@alopes
Copy link

alopes commented Nov 18, 2015

+1

1 similar comment
@joaoahmad
Copy link

+1

@buraktamturk
Copy link

+1 why this PR is not merged yet...

@internalfx
Copy link

👍

2 similar comments
@FilipeQ
Copy link

FilipeQ commented Jan 6, 2016

+1

@chuwik
Copy link

chuwik commented Jan 8, 2016

+1

@druska
Copy link

druska commented Feb 2, 2016

+1

3 similar comments
@danikenan
Copy link

+1

@utkuturunc
Copy link

+1

@robcaldecott
Copy link

+1

@mzabriskie mzabriskie merged commit 261e416 into axios:master Mar 6, 2016
@Yunnkii
Copy link

Yunnkii commented Apr 26, 2018

+1

@axios axios locked and limited conversation to collaborators May 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet