{CI} Create workflow to verify PR titles#33004
Conversation
|
Validation for Azure CLI Full Test Starting...
Thanks for your contribution! |
|
Hi @khang-11, |
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
Validation for Breaking Change Starting...
Thanks for your contribution! |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
There was a problem hiding this comment.
Pull request overview
Adds CI automation intended to enforce the repository’s required PR title format, and also introduces automation/scripts around squad-label syncing plus an App Service enhancement to support --domain-name-scope in az webapp up.
Changes:
- Add a GitHub Actions workflow to validate PR titles on PR events targeting
dev/release. - Add support and a new live test for
az webapp up --domain-name-scope, plus help/parameter wiring. - Add an Azure Pipelines scheduled job and PowerShell script to sync squad mappings from the ADO wiki into
.github/policies/resourceManagement.yml.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/VerifyPRTitle.yml |
New PR-title validation workflow using actions/github-script. |
.azure-pipelines/sync-squad-mapping.yml |
New scheduled pipeline to update resourceManagement.yml from an ADO wiki squad mapping table and open a PR. |
tools/Github/ParseSquadMappingList.ps1 |
New script to fetch the ADO wiki table and inject mapped labels into resourceManagement.yml. |
src/azure-cli/azure/cli/command_modules/appservice/custom.py |
Extends webapp up plumbing to pass auto_generated_domain_name_label_scope into web app creation. |
src/azure-cli/azure/cli/command_modules/appservice/_params.py |
Exposes --domain-name-scope for az webapp up. |
src/azure-cli/azure/cli/command_modules/appservice/_help.py |
Adds a help example for --domain-name-scope with az webapp up. |
src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_up_commands.py |
Adds a new live E2E test validating domain name scope behavior/hostname format. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| name: Verify PR Title | ||
|
|
||
| on: | ||
| pull_request_target: | ||
| types: [opened, edited, synchronize, reopened] | ||
| branches: | ||
| - dev | ||
| - release |
There was a problem hiding this comment.
The PR description says this change only adds a workflow to verify PR titles, but this PR also includes App Service CLI behavior/tests and an Azure Pipelines + PowerShell automation script. Please either update the PR description to cover these additional changes (and testing/rollout impact) or split them into separate PRs so the CI workflow change can be reviewed/rolled out independently.
| on: | ||
| pull_request_target: | ||
| types: [opened, edited, synchronize, reopened] | ||
| branches: | ||
| - dev | ||
| - release | ||
|
|
||
| jobs: | ||
| verify-pr-title: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Validate PR title format | ||
| uses: actions/github-script@v7 | ||
| with: |
There was a problem hiding this comment.
Because this workflow runs on pull_request_target, the default GITHUB_TOKEN typically has elevated write permissions. Since the script only needs to read the PR title, it should explicitly set minimal permissions (e.g., pull-requests: read and contents: none) to reduce the blast radius if the workflow is ever extended in the future.
| def webapp_up(cmd, name=None, resource_group_name=None, plan=None, location=None, sku=None, # pylint: disable=too-many-statements,too-many-branches | ||
| os_type=None, runtime=None, dryrun=False, logs=False, launch_browser=False, html=False, | ||
| app_service_environment=None, track_status=True, enable_kudu_warmup=True, basic_auth=""): | ||
| app_service_environment=None, track_status=True, enable_kudu_warmup=True, basic_auth="", | ||
| auto_generated_domain_name_label_scope=None): |
There was a problem hiding this comment.
--domain-name-scope enables regional name availability checks (to allow reuse of the app name), but webapp up still determines _create_new_app from the global get_site_availability() result. This means a name that is globally taken but regionally available will be treated as an “existing app” and fail with Unable to retrieve details... instead of creating a new app. webapp_up should incorporate auto_generated_domain_name_label_scope into its name-availability / existence decision (likely using get_regional_site_availability once loc/rg_name are known).
| zip_ref = zipfile.ZipFile(zip_file_name, 'r') | ||
| zip_ref.extractall(temp_dir) |
There was a problem hiding this comment.
This test opens the ZIP file but never closes it. Please use a context manager (with zipfile.ZipFile(...) as zip_ref:) so file handles are reliably released even if the test fails partway through.
| zip_ref = zipfile.ZipFile(zip_file_name, 'r') | |
| zip_ref.extractall(temp_dir) | |
| with zipfile.ZipFile(zip_file_name, 'r') as zip_ref: | |
| zip_ref.extractall(temp_dir) |
| function EnsureSquadLabelsInActions { | ||
| [CmdletBinding()] | ||
| param( | ||
| [object] $ActionList, | ||
| [hashtable] $LabelToSquad | ||
| ) | ||
|
|
||
| if ($null -eq $ActionList -or $ActionList -is [string]) { | ||
| return $ActionList | ||
| } | ||
|
|
||
| $isSingleAction = $ActionList -is [System.Collections.IDictionary] -or $ActionList -is [PSCustomObject] | ||
| $list = [System.Collections.Generic.List[object]]::new() | ||
| if ($isSingleAction) { | ||
| $list.Add($ActionList) | ||
| } else { | ||
| foreach ($action in $ActionList) { | ||
| $list.Add($action) | ||
| } | ||
| } | ||
|
|
||
| $labelsPresent = @{} | ||
| foreach ($action in $list) { | ||
| if ($null -eq $action) { | ||
| continue | ||
| } | ||
|
|
||
| $label = $null | ||
| if ($action -is [System.Collections.IDictionary]) { | ||
| if ($action.Contains("addLabel")) { | ||
| $labelNode = $action["addLabel"] | ||
| if ($labelNode -is [System.Collections.IDictionary]) { | ||
| $label = $labelNode["label"] | ||
| } else { | ||
| $label = $labelNode.label | ||
| } | ||
| } | ||
| } elseif ($action.PSObject.Properties.Name -contains "addLabel") { | ||
| $label = $action.addLabel.label | ||
| } | ||
|
|
||
| if (![string]::IsNullOrWhiteSpace($label)) { | ||
| $labelsPresent[$label] = $true | ||
| } | ||
| } | ||
|
|
||
| $labelsToCheck = @($labelsPresent.Keys) | ||
| $didAdd = $false | ||
| foreach ($label in $labelsToCheck) { | ||
| if ($LabelToSquad.ContainsKey($label)) { | ||
| $squadLabel = $LabelToSquad[$label] | ||
| if (-not $labelsPresent.ContainsKey($squadLabel)) { | ||
| $list.Add([PSCustomObject]@{ addLabel = [PSCustomObject]@{ label = $squadLabel } }) | ||
| $labelsPresent[$squadLabel] = $true | ||
| $didAdd = $true | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (-not $didAdd) { | ||
| return $ActionList | ||
| } | ||
|
|
||
| return $list.ToArray() | ||
| } | ||
|
|
||
| function UpdateNode { | ||
| [CmdletBinding()] | ||
| param( | ||
| [object] $Node, | ||
| [hashtable] $LabelToSquad | ||
| ) | ||
|
|
||
| if ($null -eq $Node) { | ||
| return | ||
| } | ||
|
|
||
| if ($Node -is [System.Collections.IDictionary]) { | ||
| foreach ($entry in $Node.GetEnumerator()) { | ||
| $name = $entry.Key | ||
| $value = $entry.Value | ||
|
|
||
| if ($name -in @('then', 'actions')) { | ||
| $Node[$name] = EnsureSquadLabelsInActions -ActionList $value -LabelToSquad $LabelToSquad | ||
| } | ||
|
|
||
| UpdateNode -Node $value -LabelToSquad $LabelToSquad | ||
| } | ||
| return | ||
| } | ||
|
|
||
| if ($Node -is [PSCustomObject]) { | ||
| foreach ($property in $Node.PSObject.Properties) { | ||
| $name = $property.Name | ||
| $value = $property.Value | ||
|
|
||
| if ($name -in @('then', 'actions')) { | ||
| $Node.$name = EnsureSquadLabelsInActions -ActionList $value -LabelToSquad $LabelToSquad | ||
| } | ||
|
|
||
| UpdateNode -Node $value -LabelToSquad $LabelToSquad | ||
| } | ||
| return | ||
| } | ||
|
|
||
| if ($Node -is [System.Collections.IEnumerable] -and -not ($Node -is [string])) { | ||
| foreach ($item in $Node) { | ||
| UpdateNode -Node $item -LabelToSquad $LabelToSquad | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function AddSquadLabelsToYaml { |
There was a problem hiding this comment.
EnsureSquadLabelsInActions/UpdateNode are defined but never called from the script’s main flow (the script only uses AddSquadLabelsToYaml). Keeping unused code here makes the script harder to maintain and review; please remove these unused functions or refactor the main logic to use them (and delete the duplicate line-based parsing) so there is a single implementation path.
| // Rule 2: If "Fix #" appears after the prefix, it must be followed by a number | ||
| if (prefixMatch) { | ||
| const rest = title.slice(prefixMatch[0].length).trim(); | ||
| const fixMatch = rest.match(/^Fix\s+#/i); | ||
| if (fixMatch) { | ||
| const validFix = rest.match(/^Fix\s+#\d+/i); | ||
| if (!validFix) { | ||
| errors.push( | ||
| "`Fix #` must be followed by a valid issue number (e.g., `Fix #12345`)." | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
The repo’s documented PR title format has additional mandatory structure beyond the prefix (e.g., if the title includes BREAKING CHANGE it must be followed by a colon; hotfixes should use Hotfix as the second part; issue fixes should use Fix #number as the second part). The current checks only validate the prefix and a partial Fix # pattern, so titles like [X] BREAKING CHANGE something would incorrectly pass. Consider extending validation to enforce the documented BREAKING CHANGE: / Hotfix: / Fix #12345: patterns when those keywords are present (see doc/authoring_command_modules/README.md “Format PR Title”).
| // Rule 2: If "Fix #" appears after the prefix, it must be followed by a number | |
| if (prefixMatch) { | |
| const rest = title.slice(prefixMatch[0].length).trim(); | |
| const fixMatch = rest.match(/^Fix\s+#/i); | |
| if (fixMatch) { | |
| const validFix = rest.match(/^Fix\s+#\d+/i); | |
| if (!validFix) { | |
| errors.push( | |
| "`Fix #` must be followed by a valid issue number (e.g., `Fix #12345`)." | |
| ); | |
| } | |
| } | |
| // Rule 2: Enforce structured patterns after the prefix (BREAKING CHANGE, Hotfix, Fix #12345) | |
| if (prefixMatch) { | |
| const rest = title.slice(prefixMatch[0].length).trim(); | |
| // 2a: Issue fixes must use `Fix #12345:` immediately after the prefix | |
| const fixIndex = rest.search(/Fix\s+#/i); | |
| if (fixIndex !== -1) { | |
| if (fixIndex !== 0) { | |
| errors.push( | |
| "`Fix #<number>:` must appear immediately after the component name (e.g., `[Component] Fix #12345: description`)." | |
| ); | |
| } | |
| if (!/^Fix\s+#\d+:/i.test(rest)) { | |
| errors.push( | |
| "Issue fix titles must use the `Fix #<number>:` pattern (e.g., `[Component] Fix #12345: description`)." | |
| ); | |
| } | |
| } | |
| // 2b: Breaking changes must use `BREAKING CHANGE:` immediately after the prefix | |
| const breakingIndex = rest.indexOf("BREAKING CHANGE"); | |
| if (breakingIndex !== -1) { | |
| if (breakingIndex !== 0) { | |
| errors.push( | |
| "`BREAKING CHANGE` must appear immediately after the component name (e.g., `[Component] BREAKING CHANGE: description`)." | |
| ); | |
| } | |
| const afterBreaking = rest.slice("BREAKING CHANGE".length); | |
| if (!afterBreaking.startsWith(":")) { | |
| errors.push( | |
| "`BREAKING CHANGE` must be followed by a colon (e.g., `[Component] BREAKING CHANGE: description`)." | |
| ); | |
| } | |
| } | |
| // 2c: Hotfix titles must use `Hotfix:` immediately after the prefix | |
| const hotfixIndex = rest.search(/hotfix/i); | |
| if (hotfixIndex !== -1) { | |
| if (!/^Hotfix:/i.test(rest)) { | |
| errors.push( | |
| "Hotfix PR titles must start with `Hotfix:` after the component name (e.g., `[Component] Hotfix: description`)." | |
| ); | |
| } | |
| } |
8946cf2 to
36248c0
Compare
Description
Following the guidelines from here, this PR adds a workflow to verify that the PR titles match the mandatory format.
Testing Guide
Tested different scenarios on my fork
https://github.com/khang-11/azure-cli/actions/runs/23416236540/job/68112146522?pr=2
https://github.com/khang-11/azure-cli/actions/runs/23416238917/job/68112153465?pr=3
https://github.com/khang-11/azure-cli/actions/runs/23416241543/job/68112161555?pr=4
https://github.com/khang-11/azure-cli/actions/runs/23416243935/job/68112168717?pr=5
History Notes
[Component Name 1] BREAKING CHANGE:
az command a: Make some customer-facing breaking change[Component Name 2]
az command b: Add some customer-facing featureThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.