Unity 8

Merge lp://staging/~mfrey/unity8/lock-fix into lp://staging/unity8

lock-fix
Merge into trunk

Proposed by Michael Frey on 2014-10-20

Status:

Merged

Approved by:

Michael Terry on 2014-10-21

Approved revision:

1372

Merged at revision:

1384

Proposed branch:

lp://staging/~mfrey/unity8/lock-fix

Merge into:

lp://staging/unity8

Diff against target:

13 lines (+2/-1)

1 file modified

qml/Shell.qml (+2/-1)

To merge this branch:

bzr merge lp://staging/~mfrey/unity8/lock-fix

Related bugs:

Bug #1378012: [krillin only] screen goes black immediately when pressing the Call button in dialer	High	In Progress
Bug #1378043: [TOPBLOCKER] Phone is getting locked during a call	Critical	Invalid

Link a bug report

Reviewer	Review Type	Date Requested	Status
Albert Astals Cid (community)		2014-10-20	Abstain on 2014-10-21
PS Jenkins bot (community)	continuous-integration	2014-10-20	Needs Fixing on 2014-10-20
Michael Terry		2014-10-20	Approve on 2014-10-20
Ricardo Salveti		2014-10-20	Pending
Review via email: mp+238951@code.staging.launchpad.net

This proposal supersedes a proposal from 2014-10-20.

Commit message

Added a check for Proximity to determine if we show the lock screen.

Description of the change

Added a check for Proximity to determine if we show the lock screen.
Fixes #1378012 and #1378043

Revision history for this message

Ricardo Salveti (rsalveti) wrote on 2014-10-20: Posted in a previous version of this proposal

#

LGTM

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-20: Posted in a previous version of this proposal

#

FAILED: Continuous integration, rev:1369
http://jenkins.qa.ubuntu.com/job/unity8-ci/4707/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/6067/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/1693/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/1803/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/1801/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/7319/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/4707/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2014-10-20: Posted in a previous version of this proposal

#

4/18 Test #4: whitespace .......................***Failed 1.07 sec
/tmp/buildd/unity8-8.00+14.10.20141013.2bzr1369pkg0utopic1693/qml/Shell.qml: bad whitespace in line 597

review: Needs Fixing

Revision history for this message

Michael Terry (mterry) wrote on 2014-10-20: Posted in a previous version of this proposal

#

Is this a race between callManager.hasCalls and the status change signal?

Revision history for this message

Michael Frey (mfrey) wrote on 2014-10-20: Posted in a previous version of this proposal

#

> Is this a race between callManager.hasCalls and the status change signal?

Yes.

Revision history for this message

Michael Terry (mterry) wrote on 2014-10-20: Posted in a previous version of this proposal

#

Also, in terms of style:
- Is there a reason you use a grouping parens?
- Use a triple-equal symbol like !== instead of !=
- And the if end-line continuation should look like:

if (Powerd.status === Powerd.Off && reason !== Powerd.Proximity &&
!callManager.hasCalls && !edgeDemo.running) {
foo...
}

Or maybe even for better clarity:

if (Powerd.status === Powerd.Off &&
        reason !== Powerd.Proximity &&
        !callManager.hasCalls &&
        !edgeDemo.running) {
    foo...
}

Revision history for this message

Michael Frey (mfrey) wrote on 2014-10-20: Posted in a previous version of this proposal

#

> Also, in terms of style:
> - Is there a reason you use a grouping parens?
> - Use a triple-equal symbol like !== instead of !=
> - And the if end-line continuation should look like:
>
> if (Powerd.status === Powerd.Off && reason !== Powerd.Proximity &&
> !callManager.hasCalls && !edgeDemo.running) {
> foo...
> }
>
> Or maybe even for better clarity:
>
> if (Powerd.status === Powerd.Off &&
> reason !== Powerd.Proximity &&
> !callManager.hasCalls &&
> !edgeDemo.running) {
> foo...
> }

Good catch. Fixed the style and pushed.

Revision history for this message

Michael Terry (mterry) wrote on 2014-10-20: Posted in a previous version of this proposal

#

Love it!

review: Approve

Revision history for this message

Michael Terry (mterry) wrote on 2014-10-20: Posted in a previous version of this proposal

#

(assuming that jenkins likes it too this time)

Revision history for this message

Michael Terry (mterry) wrote on 2014-10-20:

#

Still approved, but waiting for top-approval on jenkins.

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-20: Posted in a previous version of this proposal

#

FAILED: Continuous integration, rev:1370
http://jenkins.qa.ubuntu.com/job/unity8-ci/4708/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/6071/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/1694
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/1804
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/1802
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/5701/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/7323
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/7323/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/14871

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/4708/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-20:

#

FAILED: Continuous integration, rev:1372
http://jenkins.qa.ubuntu.com/job/unity8-ci/4712/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/6078/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/1698
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/1808
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/1806
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/5707/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/7330
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/7330/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/14878

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/4712/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) on 2014-10-21:

#

review: Abstain

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes:

Michael Frey

Mohammad Rakibul Hasan

Unity Team