Issue #290: Implement compatibility with new Bitbucket rest api #361
Conversation
There was a problem hiding this comment.
This PR looks good to me and is rather relevant for users of this plugin. I suggest to add one more test case (see my comments below).
@scaytrase and @sghill: My Java skills have become somewhat rusty (no pun intended) in the past years. I would appreciate either one of you give the code a looksee, as well, before merge.
| uri = String.join("/", tidyBase, "rest/build-status/1.0/commits", commit); | ||
| } | ||
| else { | ||
| uri = String.join("/", tidyBase, "rest/api/latest/projects", projectKey, "repos", slug, "commits", commit, "builds"); |
There was a problem hiding this comment.
The implementation looks fine to me. However, the tests currently cover the legacy API calls (w/o project key and repo slug), only. I suggest you add another test for the new API, as well.
|
Seconding @gruetter's request for more test coverage. I think we should also consider whether this makes sense to add as a separate notifier rather than modifying the existing one. There is support within the plugin for changing the notifier by system property, which would mean any issues with the new notifier could be easily worked around. Would love to hear more feedback about this idea. |
BUGinator86
force-pushed
the
enhancement/restapi-update
branch
from
e814699 to
1b33f18
Compare
and fix missing "parent" property in JSON for required builds.
This fixes Issue #290 without breaking compatibility with older versions of BitBucket/Stash.
We support the old deprecated REST API and the new REST API.
Testing done
The test was done in our test environment with BitBucket 7.21 that supports both versions of the API, because we needed a slim plugin for our required builds.
After both versions successfully worked in our test environment we now use it for about 3 Weeks in production without any problems.
Submitter checklist