Closed Bug 1002413 Opened 7 years ago Closed 7 years ago

[Build] Refactor webapp-zip.js

Categories

(Firefox OS Graveyard :: Gaia::Build, defect)

x86
macOS
defect
Not set
normal

Tracking

(feature-b2g:2.0)

RESOLVED FIXED
2.0 S2 (23may)
feature-b2g 2.0

People

(Reporter: gduan, Assigned: gduan)

References

Details

(Whiteboard: [p=3])

Attachments

(1 file)

1. Refactor webapp-zip.js
2. add tests
Whiteboard: [p=2]
Attached file PR to master
waiting for travis.
Comment on attachment 8415076 [details] [review]
PR to master

Hi Yuren,
could you help me to review this patch?
thanks.
Attachment #8415076 - Flags: review?(yurenju.mozilla)
Hi George, what goal do you want to achieve on this refactoring issue?
The refactoring goal is as below
1. make it more testable.
2. move xpcshell dependency to utils.
3. isolate main function (WebappZip) by webapp (see https://github.com/mozilla-b2g/gaia/pull/18768/files#diff-8d06cf2fa60a709d7fe1324030517ce8R229), so that it'll be easier to run async zip-app in the future.
Comment on attachment 8415076 [details] [review]
PR to master

After discussed with George, we can use middleware pattern to handle each job in |addToZip|, then we will have an very easy way to extend webapp-zip.

Thanks George!
Attachment #8415076 - Flags: review?(yurenju.mozilla)
Target Milestone: --- → 2.0 S2 (23may)
Whiteboard: [p=2] → [p=3]
Comment on attachment 8415076 [details] [review]
PR to master

Hi Yuren,
as offline discussed, we should let this script do webapp-zip only after bug 1008114 has landed. So, I open bug 1010095 as follow-up.

Please kindly check this patch again. Thanks.
Attachment #8415076 - Flags: review?(yurenju.mozilla)
Comment on attachment 8415076 [details] [review]
PR to master

this pr looks pretty good and after we solve bug 1010095 we will get a super clear webapp-zip :-)

good jog George!
Attachment #8415076 - Flags: review?(yurenju.mozilla) → review+
Thanks Yuren,

https://github.com/mozilla-b2g/gaia/commit/14e5ee1c03b255e4dca98756b1dae012e931a659
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
feature-b2g: --- → 2.0
You need to log in before you can comment on or make changes to this bug.