Update to latest App Center Distribution API #6987

radiantspace · 2018-04-13T16:27:49Z

Implement usage of the latest App Center Distribution API (destinations)

hashtagchris · 2018-04-13T16:41:18Z

Tasks/AppCenterDistribute/task.json

@@ -13,7 +13,7 @@
    "version": {
        "Major": 0,
        "Minor": 131,


Even if this isn't delivering a new feature, I would probably bump Minor to 134 (the current VSTS sprint), or 133 if you're going to port to the release branch, instead of bumping patch. That gives a rough indication of when the task was last modified.

Thanks for the review, bumped the version.

hashtagchris · 2018-04-13T16:45:53Z

Tasks/AppCenterDistribute/appcenterdistribute.ts

+        "release_notes": releaseNotes,
+        "destinations": [
+            {
+                "id": distributionGroupId


Does the request have to indicate what version of the API it's calling in some way? Or does the server handle both old and new style requests?

Or is the current version of the task broken?

The reasonable question - both APIs are currently available - http://openapi.appcenter.ms/#/distribute/releases_update

The version is in the in the URL /0.1/... however, we are working on moving the entire team to v1.0. In the meanwhile we will keep support both options but we will remove the old properties from the docs.

hashtagchris

Looks good.

madhurig

Changes look fine, hope you have verified with current version of task when distribution_group is set and not set.

dennispan · 2018-04-13T18:10:09Z

Tasks/AppCenterDistribute/task.json

@@ -12,7 +12,7 @@
    "author": "Microsoft Corporation",
    "version": {
        "Major": 0,
-        "Minor": 131,
+        "Minor": 134,


If we want to rename distributionGroupId to destinationId. Will this be good enough to avoid breaking current customers? Do we need to bump the major version?

@dennispan will figure that out with @madhurig and @hashtagchris. Changed the label so we can keep the current users safe and reflect it works both for groups and stores.

Renaming input names is a breaking change and will need a major version increment

If you want to rename it just for yaml without a breaking change, you can add "aliases": [ "destinationId" ] to task.json, allowing people to use either input name in yaml (and the API?). Build definitions saved in the designer will still use distributionGroupId though.

If you want to fully change the name, a breaking change is necessary.

Talked to Garrett (Distribution PM). Let's make the change and bump the major version

Done, @dennispan

izikl · 2018-04-13T20:54:20Z

Tasks/AppCenterDistribute/Strings/resources.resjson/en-US/resources.resjson

-  "loc.input.label.distributionGroupId": "Distribution group ID",
-  "loc.input.help.distributionGroupId": "ID of the distribution group the app will deploy to. Leave it empty to use the default group.",
+  "loc.input.label.distributionGroupId": "Destination ID",
+  "loc.input.help.distributionGroupId": "ID of the distribution group or store the app will deploy to. Leave it empty to use the default group.",


is it easy to change this loc.input.help.distributionGroupId and loc.input.label.distributionGroupId to loc.input.help.destinationId and loc.input.label.distributionGroupId?

Oh, yeah, just saw the discussion below.

@nevalenny I agree with @izikl that we should update the label names

Sure, it's updated automatically on a building, changed as part of the property rename.

dennispan · 2018-04-15T06:27:01Z

Looks good to me. @madhurig @hashtagchris @izikl do you want to take another look?

madhurig · 2018-04-15T12:11:29Z

Tasks/AppCenterDistribute/Strings/resources.resjson/es-es/resources.resjson

@@ -26,8 +26,8 @@
  "loc.input.help.releaseNotesInput": "Notas de esta versión.",
  "loc.input.label.releaseNotesFile": "Release notes file",
  "loc.input.help.releaseNotesFile": "Seleccione un archivo de texto con codificación UTF-8 que contenga las notas de esta versión.",
-  "loc.input.label.distributionGroupId": "Distribution group ID",
-  "loc.input.help.distributionGroupId": "ID of the distribution group the app will deploy to. Leave it empty to use the default group.",
+  "loc.input.label.destinationId": "Destination ID",


did you manually change all the resources.resjson? That is not required, only commit the one generated resources file.

Ok, will revert those, thanks for the review!

madhurig · 2018-04-15T12:15:36Z

Tasks/AppCenterDistribute/task.json

-        "Major": 0,
-        "Minor": 131,
+        "Major": 1,
+        "Minor": 134,
        "Patch": 0
    },


Add "relaseNotes" to indicate to the user whats new or different in 1.* from 0.*. E.g. https://github.com/Microsoft/vsts-tasks/blob/master/Tasks/Maven/task.json#L22

Sure, thanks!

madhurig · 2018-04-15T12:17:00Z

Looks good, left couple of small comments.

dennispan · 2018-04-16T07:21:24Z

Tasks/AppCenterDistribute/task.json

        "Patch": 0
    },
+    "releaseNotes": "Take an advantage of the new Distribution API to add the support for distribution to stores.",


I'd say something simpler like Add support for distribution to stores.

Fixed, thanks!

sergey-akhalkov

lgtm!

hashtagchris · 2018-04-16T20:38:43Z

Tasks/AppCenterDistribute/task.json

@@ -143,11 +144,14 @@
            "visibleRule": "releaseNotesSelection = file"
        },
        {
-            "name": "distributionGroupId",
+            "name": "destinationId",


Interesting - you made the old name an alias of the new name. Usually we did the reverse (without bumping the major version). There's a chance this isn't a breaking change. Do you want to test first*, or just proceed with the bump of the major version?

*Use v0 in the designer, specify a value for distributionGroupId, save the BD, switch to v1, see if your value is still there, save the BD and queue a build.

Just proceed with the major version bump

Yury Nevalenny added 3 commits April 13, 2018 18:41

Draft for new distribution destinations API usage

fd368ad

Fix for backward compatibility and better deployment experience

769af9f

Fix tests

6a7e61f

evkhramkov approved these changes Apr 13, 2018

View reviewed changes

hashtagchris reviewed Apr 13, 2018

View reviewed changes

Bump minor version

dd652d5

hashtagchris approved these changes Apr 13, 2018

View reviewed changes

madhurig approved these changes Apr 13, 2018

View reviewed changes

dennispan reviewed Apr 13, 2018

View reviewed changes

Change title to reflect usage of Destination ID

c89344e

izikl reviewed Apr 13, 2018

View reviewed changes

izikl approved these changes Apr 13, 2018

View reviewed changes

Change to destinationId and bump major version

a05b465

madhurig reviewed Apr 15, 2018

View reviewed changes

Add release note and revert manual localization updates

2a4a26a

dennispan reviewed Apr 16, 2018

View reviewed changes

Fix release note

7f1743e

sergey-akhalkov approved these changes Apr 16, 2018

View reviewed changes

hashtagchris reviewed Apr 16, 2018

View reviewed changes

hashtagchris merged commit 36a3037 into microsoft:master Apr 16, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to latest App Center Distribution API #6987

Update to latest App Center Distribution API #6987

radiantspace commented Apr 13, 2018

hashtagchris Apr 13, 2018

radiantspace Apr 13, 2018

hashtagchris Apr 13, 2018

radiantspace Apr 13, 2018

izikl Apr 13, 2018 •

edited

Loading

hashtagchris left a comment

madhurig left a comment

dennispan Apr 13, 2018 •

edited

Loading

radiantspace Apr 13, 2018

madhurig Apr 13, 2018

hashtagchris Apr 13, 2018

dennispan Apr 13, 2018

radiantspace Apr 14, 2018

izikl Apr 13, 2018

izikl Apr 13, 2018

dennispan Apr 13, 2018

radiantspace Apr 14, 2018

dennispan commented Apr 15, 2018

madhurig Apr 15, 2018

radiantspace Apr 16, 2018

madhurig Apr 15, 2018

radiantspace Apr 16, 2018

radiantspace Apr 16, 2018

madhurig commented Apr 15, 2018

dennispan Apr 16, 2018

radiantspace Apr 16, 2018

sergey-akhalkov left a comment

hashtagchris Apr 16, 2018 •

edited

Loading

dennispan Apr 16, 2018

Update to latest App Center Distribution API #6987

Update to latest App Center Distribution API #6987

Conversation

radiantspace commented Apr 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

izikl Apr 13, 2018 • edited Loading

Choose a reason for hiding this comment

hashtagchris left a comment

Choose a reason for hiding this comment

madhurig left a comment

Choose a reason for hiding this comment

dennispan Apr 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dennispan commented Apr 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

madhurig commented Apr 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sergey-akhalkov left a comment

Choose a reason for hiding this comment

hashtagchris Apr 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

izikl Apr 13, 2018 •

edited

Loading

dennispan Apr 13, 2018 •

edited

Loading

hashtagchris Apr 16, 2018 •

edited

Loading