[JENKINS-51982] Do not create install URL #51
Conversation
If public_url not available typically because we are using an API key that has insufficient permissions to create releases.
|
I don't understand the problem domain well enough to comment (and I expect I'm not the only one in the group). Perhaps an automated test would help? |
Using id can cause issues when the UploadApp method is used as the id is static and points to the id of the app rather than the version which is what we want.
|
@daniel-beck that's a fair comment. I've added additional tests to cover these scenarios. In short, if an upload API key the user is using has insufficient permissions, we should not be creating a build action as it is meaningless. The old code assumed we would always receive a field in the returned JSON response from HockeyApp. Hope the tests help in that regard. Let me know if there is anything else. |
daniel-beck
left a comment
There was a problem hiding this comment.
From the blind man on the galloping horse (as Stephen Connolly likes to say), LGTM 😃
In general this change looks straightforward. Assuming the domain is implemented correctly, there's little that can go wrong here. Are you concerned about something specific, or just want the additional pair of eyes on this change?
|
Hi *@daniel-beck* I'm a new maintainer so wanted to start slowly. An extra
pair of eyes will be the case for a lot of these PRs*. *
Thanks for the comments the suggestion of some automated tests is exactly
the thing I'm looking fo in these reviews.
Cheers.
|
|
@mezpahlan Oops, sorry about the accidental closing, got my browser tabs confused. |
If public_url not available typically because we are using an API key
that has insufficient permissions to create releases. In this scenario
installUrlresolves tonull/app_versions/{buildId}which makes no sense to be creating.@jenkinsci/code-reviewers would you do the honours, please. Thanks.