Ubuntu UI Toolkit

Merge lp://qastaging/~loic.molinari/ubuntu-ui-toolkit/ubuntu-ui-toolkit-ubuntushape-wrong-deprecation-log-fix into lp://qastaging/ubuntu-ui-toolkit/staging

ubuntu-ui-toolkit-ubuntushape-wrong-deprecation-log-fix
Merge into staging

Proposed by Loïc Molinari on 2015-11-24

Status:

Merged

Approved by:

Zsombor Egri on 2015-12-03

Approved revision:

1727

Merged at revision:

1736

Proposed branch:

lp://qastaging/~loic.molinari/ubuntu-ui-toolkit/ubuntu-ui-toolkit-ubuntushape-wrong-deprecation-log-fix

Merge into:

lp://qastaging/ubuntu-ui-toolkit/staging

Diff against target:

178 lines (+33/-27)

2 files modified

src/Ubuntu/Components/plugin/ucubuntushape.cpp (+25/-23)
src/Ubuntu/Components/plugin/ucubuntushape.h (+8/-4)

To merge this branch:

bzr merge lp://qastaging/~loic.molinari/ubuntu-ui-toolkit/ubuntu-ui-toolkit-ubuntushape-wrong-deprecation-log-fix

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Zsombor Egri		2015-11-24	Approve on 2015-12-03
PS Jenkins bot	continuous-integration		Approve on 2015-12-02
Review via email: mp+278474@code.qastaging.launchpad.net

Commit message

[UbuntuShape] Fixed deprecation logging issues.

This fix prevents logging a deprecation warning for "image", "color" and "gradientColor" properties when the import version is less than 1.3. The logging of properties used internally (through the old image wrapper) have been removed too since the user might not even have used them.

Description of the change

[UbuntuShape] Fixed deprecation logging issues.

Revision history for this message

Zsombor Egri (zsombi) wrote on 2015-11-24:

You may want to use http://bazaar.launchpad.net/~ubuntu-sdk-team/ubuntu-ui-toolkit/staging/view/head:/src/Ubuntu/Components/plugin/ucimportversionchecker_p.h to check in what version the UbuntuShape instance is used. Check UCListItem on how it is used.

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-11-24:

FAILED: Continuous integration, rev:1725
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2529/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1257/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1259/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1256/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2529/rebuild

review: Needs Fixing (continuous-integration)

lp://qastaging/~loic.molinari/ubuntu-ui-toolkit/ubuntu-ui-toolkit-ubuntushape-wrong-deprecation-log-fix updated on 2015-11-24

1725. By Loïc Molinari on 2015-11-24: [UbuntuShape] Fixed wrong deprecation log info.

Revision history for this message

Loïc Molinari (loic.molinari) wrote on 2015-11-24:

> You may want to use http://bazaar.launchpad.net/~ubuntu-sdk-team/ubuntu-ui-too
> lkit/staging/view/head:/src/Ubuntu/Components/plugin/ucimportversionchecker_p.
> h to check in what version the UbuntuShape instance is used. Check UCListItem
> on how it is used.

Done, thanks.

Revision history for this message

Zsombor Egri (zsombi) wrote on 2015-11-25:

One more thing... I do not feel you'd need to introduce a different versioning definition just for this, you could use the version encoding we have right now, I do not think there is much performance implications using BUILD_VERSION(major, minor) vs. enums. Or even better, declaring constants with V13 = BUILD_VERSION(1, 3) then using that in the tests, i.e if (m_version - V13 && !loggedOnce) would even exclude the eventual overhead provided by the BUILD_VERSION (i.e. bit-shift + bit-or)

See comments inline.

Revision history for this message

Loïc Molinari (loic.molinari) wrote on 2015-11-25:

> One more thing... I do not feel you'd need to introduce a different versioning
> definition just for this, you could use the version encoding we have right
> now, I do not think there is much performance implications using
> BUILD_VERSION(major, minor) vs. enums. Or even better, declaring constants
> with V13 = BUILD_VERSION(1, 3) then using that in the tests, i.e if (m_version
> - V13 && !loggedOnce) would even exclude the eventual overhead provided by the
> BUILD_VERSION (i.e. bit-shift + bit-or)
>
> See comments inline.

Not that I don't like the encoding (and I don't force anyone to do the same) but I did that to avoid adding 16 bits (very likely to be 32 bits if the 4 bytes boundary is reached) per instance just for that, instead of 1 bit in the already allocated padding space. The shape being the item that's used the most by apps I try to make it as compact as possible. But I can store it as U16 if you think it's not worth it.

Yes, I'm a data torturer :)

Revision history for this message

Zsombor Egri (zsombi) wrote on 2015-11-25:

> > One more thing... I do not feel you'd need to introduce a different
> versioning
> > definition just for this, you could use the version encoding we have right
> > now, I do not think there is much performance implications using
> > BUILD_VERSION(major, minor) vs. enums. Or even better, declaring constants
> > with V13 = BUILD_VERSION(1, 3) then using that in the tests, i.e if
> (m_version
> > - V13 && !loggedOnce) would even exclude the eventual overhead provided by
> the
> > BUILD_VERSION (i.e. bit-shift + bit-or)
> >
> > See comments inline.
>
> Not that I don't like the encoding (and I don't force anyone to do the same)
> but I did that to avoid adding 16 bits (very likely to be 32 bits if the 4
> bytes boundary is reached) per instance just for that, instead of 1 bit in the
> already allocated padding space. The shape being the item that's used the most
> by apps I try to make it as compact as possible. But I can store it as U16 if
> you think it's not worth it.