Closed Bug 795252 Opened 12 years ago Closed 11 years ago

B2G STK: Refactor StkCommandParamsFactory

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: allstars.chh, Assigned: gwang)

References

Details

Attachments

(4 files, 3 obsolete files)

As philikon commented in Bug 793137 comment 9, we could refactor StkCommandParamsFactory.
Assignee: nobody → allstars.chh
Assignee: allstars.chh → gwang
Attachment #790689 - Flags: review?(allstars.chh)
Attached patch part2: Refactore StkProactiveCmdHelper. (obsolete) (deleted) — Splinter Review
Attachment #790691 - Flags: review?(allstars.chh)
Attached patch part3: Modify marionette STK tests (obsolete) (deleted) — Splinter Review
Attachment #790693 - Flags: review?(allstars.chh)
Attached patch part4: Modify xpcshell STK tests (obsolete) (deleted) — Splinter Review
Attachment #790694 - Flags: review?(allstars.chh)
Attachment #790689 - Flags: review?(allstars.chh) → review+
Comment on attachment 790689 [details] [diff] [review] part1: Refactore StkCommandParamsFactory. Review of attachment 790689 [details] [diff] [review]: ----------------------------------------------------------------- I notice it should be 'Refactor', not 'Refactore'.
Attachment #790691 - Flags: review?(allstars.chh) → review+
Comment on attachment 790693 [details] [diff] [review] part3: Modify marionette STK tests Review of attachment 790693 [details] [diff] [review]: ----------------------------------------------------------------- Please remove those unneed code. ::: dom/icc/tests/marionette/test_stk_display_text.js @@ +17,3 @@ > is(command.options.userClear, expect.userClear, expect.name); > +// } > +// if (expect.isHighPriority) { Remove them if not needed. ::: dom/icc/tests/marionette/test_stk_get_inkey.js @@ +23,2 @@ > is(command.options.isYesNoRequested, expect.isYesNoRequested, expect.name); > +// } Ditto ::: dom/icc/tests/marionette/test_stk_setup_call.js @@ +13,5 @@ > is(command.typeOfCommand, icc.STK_CMD_SET_UP_CALL, expect.name); > is(command.commandQualifier, expect.commandQualifier, expect.name); > +// if (command.options.confirmMessage) { > + is(command.options.confirmMessage, expect.confirmMessage, expect.name); > +// } Ditto
Attachment #790693 - Flags: review?(allstars.chh)
Comment on attachment 790694 [details] [diff] [review] part4: Modify xpcshell STK tests Review of attachment 790694 [details] [diff] [review]: ----------------------------------------------------------------- You forgot 'hg add test_ril_worker_stk.js' on this patch
Attachment #790694 - Flags: review?(allstars.chh)
Attachment #790693 - Attachment is obsolete: true
Attachment #791108 - Flags: review?(allstars.chh)
Attachment #791108 - Attachment description: Bug795252_part3: marionette stk test. v2 → Bug 795252 - part 3: marionette stk test. v2
Attachment #791108 - Attachment description: Bug 795252 - part 3: marionette stk test. v2 → part 3: Modify marionette stk test. v2
Attachment #791108 - Attachment description: part 3: Modify marionette stk test. v2 → part3: Modify marionette stk test. v2
Attachment #790694 - Attachment is obsolete: true
Attachment #791109 - Flags: review?(allstars.chh)
Attachment #791108 - Flags: review?(allstars.chh) → review+
Attachment #791109 - Flags: review?(allstars.chh) → review+
Attachment #790691 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: