Merge into trunk : share_data_uri_string : Code : Unity 8

Status:	Rejected
Rejected by:	Andrea Cimitan on 2016-02-24
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
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Andrea Cimitan (community)			Disapprove on 2016-02-24
Michael Zanetti (community)		2016-02-19	Needs Information on 2016-02-23
PS Jenkins bot	continuous-integration		Needs Fixing on 2016-02-19
Unity8 CI Bot	continuous-integration		Approve on 2016-02-19
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.

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-02-19:

#

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

review: Approve (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-19:

#

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

review: Needs Fixing (continuous-integration)

Michael Zanetti (mzanetti) wrote on 2016-02-23:

#

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

review: Needs Information

Ken VanDine (ken-vandine) wrote on 2016-02-23:

#

> 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.

Paweł Stołowski (stolowski) wrote on 2016-02-24:

#

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'.

Andrea Cimitan (cimi) wrote on 2016-02-24:

#

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 on 2016-02-19: shareData["uri"] should be a string, not an array so lets treat it as such.
2203. By Launchpad Translations on behalf of unity-team on 2016-02-19: Launchpad automatic translations update.
2202. By Launchpad Translations on behalf of unity-team on 2016-02-18: Launchpad automatic translations update.
2201. By Launchpad Translations on behalf of unity-team on 2016-02-17: Launchpad automatic translations update.
2200. By Launchpad Translations on behalf of unity-team on 2016-02-16: Launchpad automatic translations update.

 === 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 @@
                  var transfer = peer.request();
                  if (transfer.state === ContentTransfer.InProgress) {
                      var items = new Array();
--                    for (var i = 0; i < url.length; i++) {
--                        var exportItem = exportItemComponent.createObject();
--                        exportItem.url = url[i];
--                        items.push(exportItem);
--                    }
++                    var exportItem = exportItemComponent.createObject();
++                    exportItem.url = url;
++                    items.push(exportItem);
                      transfer.items = items;
                      transfer.state = ContentTransfer.Charged;
+                 }

Unity 8

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

Commit Message

Description of the Change

Unmerged revisions

Preview Diff

Subscribers