Gaia implementation of bug 887488 (Press call once to show last called number in the dialer app)

RESOLVED FIXED

Status

RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: gtorodelvalle, Assigned: gtorodelvalle)

Tracking

unspecified
x86
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:koi+)

Details

(Whiteboard: [u=commsapps-user c=dialer p=1])

Attachments

(2 attachments)

Basically a bug to implement the user story defined in bug 887488.
Blocks: 887488
Assignee: nobody → gtorodelvalle
Whiteboard: [ucid:Comms1], [u=commsapps-user c=dialer p=1]
Created attachment 776225 [details]
Associated PR.
Attachment #776225 - Flags: review?(etienne)
Comment on attachment 776225 [details]
Associated PR.

Comments are on github, to sum up:

* the suggestion bar tests are failing

* I don't understand all the load-related changes, my guess would be that maybe it was to ease testing... if true, we definitely shouldn't too such evasive changes in the code for unit-tests.

To be clear I'm all for making clean and testable code, but I don't want a bunch of ifs in the code that make sense only when running unit tests.
Making the code testable != creating new code paths only for the tests.

* I think using the |placeholder| property on the phone number input would make the code simpler, but this is just a suggestion.

Cheers!
Attachment #776225 - Flags: review?(etienne) → review-
Whiteboard: [ucid:Comms1], [u=commsapps-user c=dialer p=1] → [u=commsapps-user c=dialer p=1]
Once bug 894926 is solved the new unit tests for this functionality will successfully pass :-)
Depends on: 894926
This blocks a koi+
blocking-b2g: --- → koi?
blocking-b2g: koi? → koi+
Comment on attachment 776225 [details]
Associated PR.

Pull request updated including Etienne's comments and the new unit tests passing, but needing a final review :-) Thanks!
Attachment #776225 - Flags: review?(francisco.jordano)
Attachment #776225 - Flags: review-
Attachment #776225 - Flags: feedback?(ferjmoreno)
PR rebased and ready for review ;-)
Thanks German, taking the second look :)
Let's see if Travis is finally also fine with it ;-) Thanks!
Comment on attachment 776225 [details]
Associated PR.

After latest changes added in to the PR (follow converstaion in GH), this looks good to me.

Thanks German!
Attachment #776225 - Flags: review?(francisco.jordano) → review+
Merged in master: https://github.com/mozilla-b2g/gaia/commit/5a1c8dd69f66c8b5a7f2e5bc0fc183992af07b44
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Attachment #776225 - Flags: feedback?(ferjmoreno)
This resulted in a regression, see bug 899063
Blocks: 899186
There is another regression this causes, see bug 898933.
Blocks: 899063
You need to log in before you can comment on or make changes to this bug.