Ubuntu SDK API

Merge lp://staging/~ubuntu-sdk-team/ubuntu-sdk-api/listsAndNameSpaces into lp://staging/ubuntu-sdk-api

  1. listsAndNameSpaces
  2. Merge into trunk
Proposed by Cris Dywan
Status: Merged
Approved by: Benjamin Zeller
Approved revision: 4
Merged at revision: 4
Proposed branch: lp://staging/~ubuntu-sdk-team/ubuntu-sdk-api/listsAndNameSpaces
Merge into: lp://staging/ubuntu-sdk-api
Prerequisite: lp://staging/~ubuntu-sdk-team/ubuntu-sdk-api/privateApiQt5.6
Diff against target: 117 lines (+35/-25)
2 files modified
apicheck/apicheck.cpp (+29/-19)
tests/qml/Extinct.Animals.api (+6/-6)
To merge this branch: bzr merge lp://staging/~ubuntu-sdk-team/ubuntu-sdk-api/listsAndNameSpaces
Reviewer Review Type Date Requested Status
ubuntu-sdk-build-bot continuous-integration Approve
Benjamin Zeller Approve
Review via email: mp+306357@code.staging.launchpad.net

Commit message

More robust list and namespace handling in apicheck

Description of the change

Forward port of the same commit to the apicheck that lives in the toolkit:

https://code.launchpad.net/~ubuntu-sdk-team/ubuntu-ui-toolkit/listsAndNameSpaces/+merge/299048

To post a comment you must log in.
Revision history for this message
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :

PASSED: Continuous integration, rev:4
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-sdk-api-ci-amd64/9/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/6663/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-sdk-api-ci-amd64/9/rebuild

review: Approve (continuous-integration)
Revision history for this message
Benjamin Zeller (zeller-benjamin) wrote :

In the Animals test at the bottom, seems there is information lost
before you had Extinct.Animals.Europe.ModernContinent: Enum and Extinct.Animals.Pangaea.ModernContinent: Enum.
Now you have Extinct.Animals.ModernContinent: Enum twice
The Europe vs Pangaea information is lost.

Also in line 40 of the patch you do : QString newQmlType(QString(types[0]->qmlTypeName()).split("/")[1].toUtf8());.
Are you sure there are always at least 2 items in the list after splitting the string?

review: Needs Fixing
Revision history for this message
Cris Dywan (kalikiana) wrote :

> In the Animals test at the bottom, seems there is information lost
> before you had Extinct.Animals.Europe.ModernContinent: Enum and
> Extinct.Animals.Pangaea.ModernContinent: Enum.
> Now you have Extinct.Animals.ModernContinent: Enum twice
> The Europe vs Pangaea information is lost.

If filed bug 1628129 to keep track of this: basically the upcoming new format will supersede this, and it's generally going to be more robust and universal. So fixing this in the original QML-ish code would only be interesting if real code were to hit it - Ubuntu UI Toolkit, currently the only project relying on this in real life, isn't affected.

> Also in line 40 of the patch you do : QString
> newQmlType(QString(types[0]->qmlTypeName()).split("/")[1].toUtf8());.
> Are you sure there are always at least 2 items in the list after splitting the
> string?

The code has been thoroughly reviewed before with Ubuntu UI Toolkit in mind, in that sense I'm pretty confident it works in production.

Revision history for this message
Benjamin Zeller (zeller-benjamin) wrote :

Ok then

review: Approve
Revision history for this message
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :

PASSED: Continuous integration, rev:4
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-sdk-api-ci-amd64/12/
Executed test runs:
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/7081/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-sdk-api-ci-amd64/12/rebuild

review: Approve (continuous-integration)

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: