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)
Attachment #790691 - Flags: review?(allstars.chh)
Attachment #790693 - Flags: review?(allstars.chh)
Attached patch part4: Modify xpcshell STK tests (obsolete) — 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: