Conversation
|
Validation for Breaking Change 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. |
|
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>
|
|
|
@khang-11 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
There was a problem hiding this comment.
Pull request overview
Adds a lightweight CI check to validate pull request titles against a bracketed component-prefix convention, aiming to improve PR consistency and adherence to extension repo contribution guidelines.
Changes:
- Introduce a
verify_pr_titlePython module (extractors/rules/verifiers + CLI runner) to validate PR title format. - Define default rules requiring a leading
[Component]or{Component}prefix and a non-empty description (with optional keywords). - Add a GitHub Actions workflow to run the check on PR open/edit/synchronize events.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/ci/verify_pr_title/verifiers.py |
Adds verifier abstractions and regex/non-empty validation helpers. |
scripts/ci/verify_pr_title/extractors.py |
Adds title extractors for full title, prefix, and post-prefix text. |
scripts/ci/verify_pr_title/rule.py |
Defines the Rule dataclass tying extractor + verifier. |
scripts/ci/verify_pr_title/rules.py |
Establishes the default PR title validation rules and error messages. |
scripts/ci/verify_pr_title/runner.py |
Implements the rule-evaluation loop returning failures. |
scripts/ci/verify_pr_title/__main__.py |
Provides a CLI entrypoint for running validation in CI. |
scripts/ci/verify_pr_title/__init__.py |
Exposes module API surface via imports and __all__. |
.github/workflows/VerifyPRTitle.yml |
Adds the GitHub Actions workflow that runs the validator on PR events. |
| name="Component prefix present", | ||
| extractor=FullTitleExtractor(), | ||
| verifier=RegexVerifier( | ||
| pattern=r"^(\[.+?\]|\{.+?\})", |
There was a problem hiding this comment.
The component-prefix regex allows whitespace-only components (e.g. "[ ] Fix ...") because it uses ".+?". This contradicts the error message claiming the bracket must be non-empty. Tighten the pattern to require at least one non-whitespace character inside the brackets so whitespace-only prefixes are rejected.
| pattern=r"^(\[.+?\]|\{.+?\})", | |
| pattern=r"^(\[(?!\s*\])[^]]+\]|\{(?!\s*\})[^}]+\})", |
| _PATTERN = re.compile(r"^(\[.+?\]|\{.+?\})") | ||
|
|
||
| def extract(self, title: str) -> str: | ||
| m = self._PATTERN.match(title) | ||
| return m.group(1) if m else "" | ||
|
|
||
|
|
||
| class AfterPrefixExtractor(Extractor): | ||
| _PATTERN = re.compile(r"^(?:\[.+?\]|\{.+?\})\s*(.*)", re.DOTALL) | ||
|
|
There was a problem hiding this comment.
The extractor patterns for the component prefix (both ComponentPrefixExtractor and AfterPrefixExtractor) also accept whitespace-only bracket content due to ".+?". If you tighten the validation regex to require a non-whitespace character inside the brackets, these patterns should be updated in sync to avoid inconsistencies between extraction and validation.
| from .rules import RULES | ||
|
|
||
|
|
||
| def run(title: str, rules: list[Rule] = RULES) -> list[tuple[str, str]]: |
There was a problem hiding this comment.
run() uses a module-level list (RULES) as a default argument. Even if you don’t mutate it today, this makes the function susceptible to accidental external mutation and harder to reason about in tests. Prefer a None default and assign RULES inside the function.
| def run(title: str, rules: list[Rule] = RULES) -> list[tuple[str, str]]: | |
| def run(title: str, rules: list[Rule] | None = None) -> list[tuple[str, str]]: | |
| if rules is None: | |
| rules = RULES |
| "[Component] (BREAKING CHANGE: | Hotfix[:] | Fix #<N>[:])? <description>\n" | ||
| "{Component} (BREAKING CHANGE: | Hotfix[:] | Fix #<N>[:])? <description>" |
There was a problem hiding this comment.
The printed "Expected format" string implies a required space after the prefix, but the current extraction/validation logic allows titles like "[Core]Fix #123: ..." (no space). Either enforce the space in the rules or adjust _EXPECTED_FORMAT so the CLI output matches what is actually accepted.
| "[Component] (BREAKING CHANGE: | Hotfix[:] | Fix #<N>[:])? <description>\n" | |
| "{Component} (BREAKING CHANGE: | Hotfix[:] | Fix #<N>[:])? <description>" | |
| "[Component](BREAKING CHANGE: | Hotfix[:] | Fix #<N>[:])? <description>\n" | |
| "{Component}(BREAKING CHANGE: | Hotfix[:] | Fix #<N>[:])? <description>" |
| for line in error.splitlines(): | ||
| print(f" {line}") | ||
| print() | ||
| print(f"Expected format:\n {_EXPECTED_FORMAT}") |
There was a problem hiding this comment.
When printing Expected format, only the first line is indented; the second line starts at column 0 because _EXPECTED_FORMAT contains a newline. Consider formatting the output by splitting into lines and indenting each line for consistent readability.
| print(f"Expected format:\n {_EXPECTED_FORMAT}") | |
| expected_format_indented = "\n".join(f" {line}" for line in _EXPECTED_FORMAT.splitlines()) | |
| print(f"Expected format:\n{expected_format_indented}") |
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally? (pip install wheel==0.30.0required)For new extensions:
About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update
src/index.jsonautomatically.You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify
src/index.json.