-
-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use DevOps API version 5.0 where possible, for maximum compatibility #1425
base: main
Are you sure you want to change the base?
Use DevOps API version 5.0 where possible, for maximum compatibility #1425
Conversation
Ah, interesting, I didn't test re-approvals. Do you think this could be resolved by using 7.1 for the approval API call and 5.0 for all other API calls? |
According to #1069, we need to re-approve the PR if automatic approval is enabled and the PR is updated (e.g., resolving merge conflicts). For the longest, I have not considered auto-approval a good practice because when used with auto-merge it can result in some unwanted mergers. However, for organizations that use GitFlow (common with Azure DevOps as compared to GitHub Flow), the updates would be targeted towards a "develop" branch that would be tested as a whole later. This is why I allowed it. Having "User approved this pull request" messages every time the PR is updated is not a problem because that indicates that it was done for the new changes. It would also be necessary when the policy requires it. Side note: @SeMuell is a sponsor so I am inclined to keep a feature that works for them intact. |
I do agree with this. However, I also think the on-premise server owners who might be prevented from upgrading promptly each year or so that Microsoft actually updates the server edition would appreciate a longer compatibility window; especially the ones that might have to factor in other organizational constraints before they can upgrade. Given that this can be done in a single line of code, I think it is at least worth considering. If this was a case of writing a new client to handle different API versions, I'd completely agree to just support the latest API version. I've updated this PR with a change that I think balances fully-featured and maximum compatibility, back to DevOps Server 2019 (API 5.0). Let me know your thoughts. It will use API version 7.1 for auto-approval, version 5.0 for everything else. If you still aren't comfortable with this change, I'll abandon it and open a separate PR to fix the
Make sense, I wasn't really thinking about this when I implemented it; I have corrected it in this PR too. |
Fixes #1423 and #1069 in TaskV2.
This change downgrades the DevOps REST API version requirement from 7.1 to 5.0, for maximum compatibility with older DevOps servers. The only exception to this is that
approvePullRequest
will use API version 7.1 in order to use theisReapprove
parameter.API version 5.0 is the same version used in dependabot-core's Azure API client.
AzureDevOpsWebApiClient
has been retested on 5.0, all APIs work as expected. See logs below where 5.0 is used to create the PR, 7.1 is used to auto-approve the PR.If the pull request is updated by Dependabot, it is also reapproved: