Users/lukillgo/googleapi v3 #134
Conversation
Coverted all tasks to use V3
Updated vsts-task-lib to azure-pipelines-task-lib Added tests to rollout update task
(which fixed runtime errors on the agent, the version of Node isn't properly supported) Updated Task UI to have rollout checkbox Minor improvements
Updated tests.
| @@ -0,0 +1,227 @@ | |||
| // common code shared by all tasks | |||
There was a problem hiding this comment.
This file is duplicated 3x. There may be a way to share the code across tasks if I demand a higher version of Node. I may look into that as a future change.
There was a problem hiding this comment.
I don't understand why you can't share the code due to the node version, can you summarize? Can you pull just the interface types out into a .d.ts file in a common location?
There was a problem hiding this comment.
The pattern we use for sharing common code is to create a 'common package'. See this for some examples. When I refactored to use with that pattern, one of the modules I included required that I upgrade the version of TypeScript that's used (why? I don't really know), which cascaded into a couple of errors that showed up only during runtime, which I eventually traced to this issue. (That's over-simplifying things as there were a half-dozen other errors to work through to get to that one.) For whatever reason, using googleapis with a common library requires bumping the version of TypeScript that gets used, which requires many of the dependencies to bump their versions, which leads to a series of runtime errors.
To answer your question, there's likely a way to use another pattern and share code, but exactly what that way should be is not really clear to me. After much time spent trying to get it to work the 'right' way, I downshifted to code duplication. It was already duplicated across all the tasks anyway, so this isn't 'worse' than before.
The existing build scripts don't seem to me to allow a common build folder to be included, but if you have a suggestion on how to do it, I'd be happy to hear it.
There was a problem hiding this comment.
Gotcha (mostly). I'll take your word for it, and I agree, no worse than where it was at before.
There was a problem hiding this comment.
'mostly' ... pretty much me too. :)
There was a problem hiding this comment.
I'd argue that this is better than a "common" folder. We've had a lot of issues with that in the other tasks repo as it makes it harder to update individual tasks that all rely on the same common files (since a common folder has no versioning)
| "googleapis": "^2.1.5", | ||
| "vsts-task-lib": "2.1.0" | ||
| "azure-pipelines-task-lib": "2.8.0", | ||
| "gaxios": "1.8.0", |
There was a problem hiding this comment.
This gaxios include fixes the build issues I was having when this task ran on Node 6.10.x
Tasks/google-play-promote/Strings/resources.resjson/en-US/resources.resjson
Outdated
Show resolved
Hide resolved
| @@ -14,7 +14,7 @@ | |||
| ], | |||
| "version": { | |||
| "Major": "1", | |||
There was a problem hiding this comment.
The other tasks had a major bump, is this okay to just have a minor bump?
There was a problem hiding this comment.
Yes, this one was already using V3 of the google APIs (a issue from last year), so the UI didn't need to change. So, a minor version is all that's necessary.
There was a problem hiding this comment.
So, I released that there is a gap in functionality that required me to update this afterall. We need a way to increase the rollout fraction in any track, so I added 'track' and bumped the version here.
Added a few more L0's
Have all the tasks send any existing release notes
Don't call updateGlobalParams as a side effect
Bumped the version for Google Play Promote and Google Play Release to accommodate the change in V3 of the API. Each 'track' now can have its own percentage of users. Release does the initial upload.
Promote moves from track-to-track. Rollout changes the percentage within a track.
I added tests to this project, bumped the versions of packages to reduce vulnerabilities, and tried to pull out common code into a submodule.
Fixes #129.