935088 - [Download Manager] Download list implementation

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Cristian Rodriguez (:crdlc)]

Implement the controller to display the list of downloads

Comment 1

[Francisco Jordano [:arcturus] [:francisco]]

Attachment: Download list implementation

I'm adding this patch to keep the flow, but I was the reviewer.

Comment 2

[Francisco Jordano [:arcturus] [:francisco]]

Landed:

https://github.com/crdlc/gaia/commit/b1bf741979e86a00f4b7547e40ea10fdf4fe0211

Comment 3

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8335931 [details] [review]
Download list implementation

Hi kaze, this is the continuation of bug 935087. Actually you need it to try this one.

As we got bug 935087, back out we decided to split the development on a separate branch to land the whole functionality in settings.

You also will need to compile with WIP4 patch for the download api: bug 938023

Also we added a mock to display downloads (all kind of possible states) even if no api is present, for better review.

Thanks!

Comment 4

[Fabien Cazenave [:kaze]]

Hmm, are you asking me to review a patch that is already landed?

Comment 5

[Fabien Cazenave [:kaze]]

Nevermind, that’s a development branch. Looking again.

Comment 6

[Borja Salguero [:borjasalguero]]

Attachment: Download List patch

Comment 10

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8340215 [details] [diff] [review]
Download List patch

r+ once comments on github addressed (that i think you are doing meanwhile I write this ;))

Comment 12

[Borja Salguero [:borjasalguero]]

Comments addressed! :)

Kaze, we are ready to get your final review! This is important because we need to land this patch for unlock all dependencies and accomplish our goal of landing this week! Could you take a look? Thanks!

Comment 13

[Borja Salguero [:borjasalguero]]

Comment on attachment 8340215 [details] [diff] [review]
Download List patch

Hi Evelyn!! After first round of feedback&review from :kaze and :arcturus, we need the final review! :)

This is a patch for landing 'Downloads' feature to Settings (System & Notifications patches are already landed in Gaia master).

We are waiting for the final icon, but it will be added in a separate PR:
https://bugzilla.mozilla.org/show_bug.cgi?id=935249

For testing is simple! Go to the browser, search for a file to download... and that's it! There are some issues to be fixed in Gecko, but :fabrice is working on it. If you have any doubt don't hesitate to ask! Thanks! 谢谢! Gracias!

Comment 14

[Fabien Cazenave [:kaze]]

Comment on attachment 8340215 [details] [diff] [review]
Download List patch

The code looks good to me, the tests are green, the Settings app still works, the startup time has not regressed, the device has not exploded: I guess it’s an r+. :-)

Thanks for the good work Borja!

Comment 15

[Borja Salguero [:borjasalguero]]

Thanks Kaze!!! I've just squashed all changes and wait till Travis for merging. Thanks a lot! Muchísimas gracias ;)

Comment 17

[Evelyn Hung]

Sorry I didn't finish my review, but the refined patch looks good to me. :)
@kaze, thanks for taking this review. :)