Merge lp:~ken-vandine/unity8/share_data_uri_string into lp:unity8

Proposed by Ken VanDine
Status: Rejected
Rejected by: Andrea Cimitan
Proposed branch: lp:~ken-vandine/unity8/share_data_uri_string
Merge into: lp:unity8
Diff against target: 18 lines (+3/-5)
1 file modified
qml/Dash/Previews/PreviewSharing.qml (+3/-5)
To merge this branch: bzr merge lp:~ken-vandine/unity8/share_data_uri_string
Reviewer Review Type Date Requested Status
Andrea Cimitan (community) Disapprove
Michael Zanetti (community) Needs Information
PS Jenkins bot (community) continuous-integration Needs Fixing
Unity8 CI Bot continuous-integration Approve
Review via email: mp+286676@code.launchpad.net

Commit message

shareData["uri"] should be a string, not an array so lets treat it as such.

Description of the change

shareData["uri"] should be a string, not an array so lets treat it as such.

To post a comment you must log in.
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:2204
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/443/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/598
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay/187
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial/187
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/621
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/639
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/639
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/635
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/635/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/635
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/635/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/635
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/635/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/635
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/635/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/635
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/635/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/635
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/635/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/443/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:2204
http://jenkins.qa.ubuntu.com/job/unity8-ci/7375/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/6575
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/790/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/2080
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/783
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1975
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1975
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/782
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/781
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/5005
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6586
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6586/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/27770
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-xenial-touch/406/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/788
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/788/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/27769

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

Why? Shouldn't we allow to share multiple files?

review: Needs Information
Revision history for this message
Ken VanDine (ken-vandine) wrote :

> Why? Shouldn't we allow to share multiple files?

I think the API current looks like it expects a single file, which is what caused the issue. shareData["uris"] would be more appropriate if we want an array instead of a string.

Revision history for this message
Paweł Stołowski (stolowski) wrote :

As discussed on IRC (with mzanetti and cimi) this should follow the documentation in scopes API:
"uri: A single URI to share or an array of URIs."

In 99% of cases scopes will be interested in sharing a single item, so we should keep the name 'uri'.

Revision history for this message
Andrea Cimitan (cimi) wrote :

Thanks ken, I did similar changes here while supporting also arrays
https://code.launchpad.net/~cimi/unity8/preview-sharing-string-and-array

review: Disapprove

Unmerged revisions

2204. By Ken VanDine

shareData["uri"] should be a string, not an array so lets treat it as such.

2203. By Launchpad Translations on behalf of unity-team

Launchpad automatic translations update.

2202. By Launchpad Translations on behalf of unity-team

Launchpad automatic translations update.

2201. By Launchpad Translations on behalf of unity-team

Launchpad automatic translations update.

2200. By Launchpad Translations on behalf of unity-team

Launchpad automatic translations update.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'qml/Dash/Previews/PreviewSharing.qml'
--- qml/Dash/Previews/PreviewSharing.qml 2015-12-17 15:42:39 +0000
+++ qml/Dash/Previews/PreviewSharing.qml 2016-02-19 15:31:32 +0000
@@ -77,11 +77,9 @@
77 var transfer = peer.request();77 var transfer = peer.request();
78 if (transfer.state === ContentTransfer.InProgress) {78 if (transfer.state === ContentTransfer.InProgress) {
79 var items = new Array();79 var items = new Array();
80 for (var i = 0; i < url.length; i++) {80 var exportItem = exportItemComponent.createObject();
81 var exportItem = exportItemComponent.createObject();81 exportItem.url = url;
82 exportItem.url = url[i];82 items.push(exportItem);
83 items.push(exportItem);
84 }
85 transfer.items = items;83 transfer.items = items;
86 transfer.state = ContentTransfer.Charged;84 transfer.state = ContentTransfer.Charged;
87 }85 }

Subscribers

People subscribed via source and target branches