Pull request updater for azure client #3153
Conversation
| end | ||
|
|
||
| def temp_branch_name | ||
| "#{source_branch_name}-temp" |
There was a problem hiding this comment.
Would it be worth generating a short id, e.g. #{source_branch_name}-temp-Digest::SHA1.hexdigest(source_branch_name)[0..6]?
Thinking of the case where two jobs try to update the same PR or someone has stashed some changes in a -temp version of the current branch, in which case it would be deleted. Seems unlikely but have a feeling I've created branches with temp suffixes in the past when wrangling a rebase for example.
There was a problem hiding this comment.
@feelepxyz Digest::SHA1.hexdigest(source_branch_name) produces consistent hashing. So for the case where we have two jobs trying to update the same PR, this might still result in issues right? I was thinking of using uuid (documentation) for uniquely identifying the temp branch. Thoughts?
There was a problem hiding this comment.
Ah yes you're right, SHA1 is stable, uuid seems like a good candidate 👍
There was a problem hiding this comment.
Cool made an update accordingly
Cool Thanks for reviewing it so promptly. Will make the suggested changes |
| source.organization + "/" + source.project + | ||
| "/_apis/git/pullrequests/" + pull_request_id) | ||
|
|
||
| JSON.parse(response.body) |
There was a problem hiding this comment.
Not blocking for now, but we might want to handle non-200 responses here
There was a problem hiding this comment.
I see we have some sort of handling in the get method of the azure client. Can you specify the scenarios that we might want to cover here?
|
Hi, can we complete this PR? I actually want to use this code further in our service. Let me know if i need to add anything else. @jurre @feelepxyz |
|
Cool Thanks! |
v0.135.0, 4 March 2021 * Gracefully handle cargo version conflicts #3213 * Pull request updater for azure client #3153 (@milind009) [v0.134.2...v0.135.0-release-notes](v0.134.2...v0.135.0-release-notes)
Feature
Added support for updating pull requests for
azure client (AzureDevOps). For updating pull requests (i.e pushing new file changes) implemented the following logic -PR & PR source branchexists. If no, return nil else continueCaveat- Currently the AzureDevOps PR files diff UI shows the diff in commits instead of the actual diff in the file content. Hence, there is a temporary workaround to push file changes to the source branch to ensure the reviewer sees the exact files changes. The workaround is as follows -Testing
Added unit tests for the pull request updater implementation for azure client as well as the additional methods added in the azure client class.