Assign sync prs #1043
Assign sync prs #1043
Conversation
chidozieononiwu
force-pushed
the
AssignSyncPRs
branch
from
769c66f to
ccc5c58
Compare
| -GitHubUsers "$(${{ parameters.GHReviewersVariable }})" | ||
| -GitHubTeams "$(${{ parameters.GHTeamReviewersVariable }})" | ||
| -PRNumber "$(Submitted.PullRequest.Number)" | ||
| -UserReviewers "$(${{ parameters.GHReviewersVariable }})" |
There was a problem hiding this comment.
| -UserReviewers "$(${{ parameters.GHReviewersVariable }})" | |
| -UserReviewers "$env:${{ parameters.GHReviewersVariable }}" |
| -GitHubTeams "$(${{ parameters.GHTeamReviewersVariable }})" | ||
| -PRNumber "$(Submitted.PullRequest.Number)" | ||
| -UserReviewers "$(${{ parameters.GHReviewersVariable }})" | ||
| -TeamReviewers "$(${{ parameters.GHTeamReviewersVariable }})" |
There was a problem hiding this comment.
| -TeamReviewers "$(${{ parameters.GHTeamReviewersVariable }})" | |
| -TeamReviewers "$env:${{ parameters.GHTeamReviewersVariable }}" |
| -PRNumber "$(Submitted.PullRequest.Number)" | ||
| -UserReviewers "$(${{ parameters.GHReviewersVariable }})" | ||
| -TeamReviewers "$(${{ parameters.GHTeamReviewersVariable }})" | ||
| -Assignees "$(${{ parameters.GHAssignessVariable }})" No newline at end of file |
There was a problem hiding this comment.
| -Assignees "$(${{ parameters.GHAssignessVariable }})" | |
| -Assignees "$env:${{ parameters.GHAssignessVariable }}" |
| $baseURI = "https://api.github.com/repos" | ||
| function SplitMembers ($membersString) | ||
| { | ||
| return @($membersString.Split(",") | % { $_.Trim() } | ? { return $_ }) |
There was a problem hiding this comment.
I'd suggest adding a check for empty like:
if (!$memberString) { return $null }
That prevents any cases were we have an empty/null value and turn it into and empty array.
| function AddReviewers ($prNumber, $users, $teams) { | ||
| if (!$users -and !$teams) | ||
| { | ||
| LogWarning "No users or teams provided for addition, exiting." |
There was a problem hiding this comment.
I don't think this should be a warning, as we don't always pass in reviewers to this script. We should simply make it a no-op.
| function AddLabelsAndOrAssignees ($prNumber, $labels, $assignees) { | ||
| if (!$labels -and !$assignees) | ||
| { | ||
| LogWarning "No labels or assignees provided for addition, exiting." |
There was a problem hiding this comment.
I don't suggest we log a warning here. Just make it a noop or a standard log message.
| [string]$Assignees | ||
| ) | ||
|
|
||
| . "${PSScriptRoot}\logging.ps1" |
There was a problem hiding this comment.
So far we only need the logging logic here, so I did not feel the need to import common.
There was a problem hiding this comment.
OK. I'm fine with this for now but at some point we might want to consider always importing common.ps1 just to avoid any differences for scripts we use in our eng-system. The only time we should avoid it is if we want to limit our dependencies in a script to use it in other contexts.
| if ($resp.Count -gt 0) { | ||
| Write-Host -f green "Pull request already exists $($resp[0].html_url)" | ||
| try { | ||
| LogDebug "Pull request already exists $($resp[0].html_url)" |
There was a problem hiding this comment.
We should keep this a normal log message otherwise it will not show up in Devops logging unless you set System.Debug = true
There was a problem hiding this comment.
| -Body ($data | ConvertTo-Json) | ||
|
|
||
| $resp | Write-Verbose | ||
| LogDebug "Pull request created https://github.com/$RepoOwner/$RepoName/pull/$($resp.number)" |
| if ($isDevOpsRun) | ||
| { | ||
| Write-Host "##vso[task.LogIssue type=debug;]$args" | ||
| Write-Host "[debug]$args" |
There was a problem hiding this comment.
Oh on my other comments I assume this was still debug logging. I think I would keep this as is so we have support for that debug logging. Perhaps we need a LogInfo or something similar that will just do a Write-Host.
chidozieononiwu
force-pushed
the
AssignSyncPRs
branch
from
ccc5c58 to
d748305
Compare
|
@chidozieononiwu I assume this is outdated based on your other PR work and there is still some outstanding feedback so I'm not going to review this further until you tell me it is ready for review again. |
chidozieononiwu
added
EngSys
|
Replaced by #1091 |
…een preview 3 and 4. (Azure#1043) `az_allocator_context` was added after preview3 shipped so renaming it to `az_span_allocator_context` isn't breaking.
2 participants
Get creator of Tools PR
Add the Tools PR creator alias as a reviewer and assignee.
Move logic for adding members (labels, reviewers and assignees) into the
eng\common\scripts\Submit-PullRequest.ps1sctiptUpdate
logging.ps1