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

Improve download_progress example #283

Merged
merged 9 commits into from
Feb 12, 2021
Merged

Improve download_progress example #283

merged 9 commits into from
Feb 12, 2021

Conversation

Folyd
Copy link
Contributor

@Folyd Folyd commented Apr 10, 2020

It seems like we have no example to demonstrate the powerful strength of Subscription.batch() API. So this is why I add this example. I know we already have a nice download example that has some duplicate code with this one. I still think we deserve such an example to instruct the new user to handle multiple asynchronous commands with Subscription.batch(). Maybe we can merge two download examples into a single one and distinct with different modes (simple and advanced)?

image

@hecrj hecrj added the improvement An internal improvement label Apr 11, 2020
@hecrj hecrj added this to the 0.2.0 milestone Apr 13, 2020
@hecrj
Copy link
Member

hecrj commented Apr 14, 2020

Looks cool! 🎉

I think we should add progress bars and merge the example with the download_progress one.

@Folyd Folyd changed the title New example: advanced_download Improve download_progress example Apr 17, 2020
@Folyd
Copy link
Contributor Author

Folyd commented Apr 17, 2020

@hecrj I have added progress bar and replaced the old download example with this one. Is it ok to be merged?

Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

Looks great!

Before merging, I believe we need to focus on simplifying the example a bit.

examples/download_progress/src/download.rs Show resolved Hide resolved
examples/download_progress/src/download.rs Outdated Show resolved Hide resolved
examples/download_progress/src/download.rs Outdated Show resolved Hide resolved
examples/download_progress/src/download.rs Outdated Show resolved Hide resolved
examples/download_progress/src/main.rs Show resolved Hide resolved
examples/download_progress/src/main.rs Outdated Show resolved Hide resolved
examples/download_progress/src/main.rs Outdated Show resolved Hide resolved
examples/download_progress/src/main.rs Outdated Show resolved Hide resolved
@Folyd
Copy link
Contributor Author

Folyd commented Apr 27, 2020

@hecrj Thanks for reviewing. :)

@hecrj hecrj modified the milestones: 0.2.0, 0.3.0 Nov 26, 2020
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

Thank you for this 🎉

I have rebased and simplified the example a bit while keeping the ProgressBar widget. I think we can merge this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement An internal improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants