880333 - [STK] "STK_CMD_SET_UP_IDLE_MODE_TEXT" code refactoring

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Noemí Freire (:noemi)]

STK code refactoring moving "STK_CMD_SET_UP_IDLE_MODE_TEXT" code from settings to system

Comment 1

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Attachment: Moved SETUP IDLE MODE TEXT to system

This patch is the first one of the STK refactoring process explained in 875679 so this patch also contains the skeleton for the rest of ICC commands to be moved to System

Comment 2

[Fabien Cazenave [:kaze]]

Comment on attachment 763435 [details]
Moved SETUP IDLE MODE TEXT to system

The Settings part looks OK to me (= code removal) but please address my nitpicks. My main objection would be to finish bug 881672 first so we could use a proper DUMP() method instead of defining yet another debug() one.

Clearing the review flag. Feel free to feedback? me when my comments are addressed, but I think Tim would be more qualified for a review since it’s mostly a System patch now.

Comment 3

[Alex Keybl [:akeybl]]

Milestoning for QE3 (due by 6/24), since this blocks bug 875679 which blocks 865985 (QE3 milestoned).

Comment 4

[Noemí Freire (:noemi)]

Notice that Bug 881672 needs to be uplifted to v1-train in order to enable STK/ICC testing

Comment 5

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 763435 [details]
Moved SETUP IDLE MODE TEXT to system

I leave comments about that DUMP() without read bug 881672.

One small nit: please add function names to all functions so we could see them in the stack trace and profiling.

Comment 6

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #5)
> Comment on attachment 763435 [details]
> Moved SETUP IDLE MODE TEXT to system
> 
> I leave comments about that DUMP() without read bug 881672.
> 
> One small nit: please add function names to all functions so we could see
> them in the stack trace and profiling.

Ok, thanks Tim. I'll add function names and merge it.

Comment 7

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Landed: https://github.com/mozilla-b2g/gaia/commit/e2a1b8437a8b100882e6c7b2f1f0780bfafbf8c1

Comment 8

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1-train.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1-train, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1-train
  git cherry-pick -x -m1 e2a1b8437a8b100882e6c7b2f1f0780bfafbf8c1
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 9

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

No dependencies. Conflict solved and uplifted:

https://github.com/mozilla-b2g/gaia/commit/d0a7727176137b7255d5f12731011c59048f4003

(In reply to John Ford [:jhford] -- please use 'needinfo?' instead of a CC from comment #8)
> I was not able to uplift this bug to v1-train.  If this bug has dependencies
> which are not marked in this bug, please comment on this bug.  If this bug
> depends on patches that aren't approved for v1-train, we need to re-evaluate
> the approval.  Otherwise, if this is just a merge conflict, you might be
> able to resolve it with:
> 
>   git checkout v1-train
>   git cherry-pick -x -m1 e2a1b8437a8b100882e6c7b2f1f0780bfafbf8c1
>   <RESOLVE MERGE CONFLICTS>
>   git commit

Comment 10

[John Ford [:jhford] CET/CEST Berlin Time]

1.1hd: d0a7727176137b7255d5f12731011c59048f4003