Add CI enforcement of Chronus change descriptions for SDK packages#45803
Add CI enforcement of Chronus change descriptions for SDK packages#45803
Conversation
Co-authored-by: l0lawrence <100643745+l0lawrence@users.noreply.github.com>
Co-authored-by: l0lawrence <100643745+l0lawrence@users.noreply.github.com>
|
@copilot this should be run for any packages touched that have a pyproject.toml |
… pyproject.toml Co-authored-by: l0lawrence <100643745+l0lawrence@users.noreply.github.com>
Updated the Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot can we try changing something in azure-keyvault-keys and azure-mgmt-eventgrid pkgs to test this ci change |
…erify chronus-verify CI workflow Co-authored-by: l0lawrence <100643745+l0lawrence@users.noreply.github.com>
Added test comments (commit
Both changes are Python source files, which are NOT excluded by chronus's Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot Run npx chronus verify |
Co-authored-by: l0lawrence <100643745+l0lawrence@users.noreply.github.com>
Fixed in commit Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot add documentation under doc/dev that advises on how to use chronus |
The documentation already exists in the repo at |
|
Discussion: How will this interact/impact [auto-prs] for mgmt-* pkgs, do we want this to trigger on them? Should we put in a default chronus entry in the auto-pr as well that says "generation" or uses the commits from the rest-api-specs pr to create the chronus entry? |
There was a problem hiding this comment.
Pull request overview
Adds GitHub Actions CI enforcement for Chronus change descriptions by running chronus verify on pull requests that modify files under sdk/*/*/**, leveraging the existing .chronus/config.yaml to decide when a change description is required.
Changes:
- Added a new GitHub Actions workflow to run Chronus verification on relevant PRs.
- Added “CI test” marker comments in one dataplane file and one mgmt file to exercise the workflow trigger.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
.github/workflows/chronus-verify.yml |
New workflow to run Chronus verification on PRs touching SDK package directories. |
sdk/keyvault/azure-keyvault-keys/azure/keyvault/keys/_sdk_moniker.py |
Adds a CI-test comment intended to trigger the workflow. |
sdk/eventgrid/azure-mgmt-eventgrid/azure/mgmt/eventgrid/_patch.py |
Replaces copyright header line with a CI-test comment to trigger the workflow. |
| paths: | ||
| - "sdk/*/*/**" | ||
|
|
There was a problem hiding this comment.
The PR description says enforcement is scoped to SDK packages that use pyproject.toml / dataplane packages, but this workflow triggers on any changes under sdk///** (including mgmt packages). Either narrow the paths filter (e.g., to pyproject.toml changes) or update the PR description/intent to reflect that mgmt packages will also be subject to Chronus verification (per .chronus/config.yaml’s sdk// package glob).
| # Copyright (c) Microsoft Corporation. | ||
| # Licensed under the MIT License. | ||
| # ------------------------------------ | ||
| # CI test: verifying chronus-verify workflow triggers for dataplane packages |
There was a problem hiding this comment.
Remove this CI-test-only comment. This module is part of the shipped library surface (even if internal), and leaving workflow-validation notes in product code adds long-lived noise and complicates future diffs/blame. Prefer validating the workflow via a PR that only touches workflow/config files (or use workflow_dispatch) rather than adding sentinel edits in SDK source.
| # CI test: verifying chronus-verify workflow triggers for dataplane packages |
| # CI test: verifying chronus-verify workflow triggers for mgmt packages | ||
| # |
There was a problem hiding this comment.
This change removes the file’s copyright header and replaces it with a CI-test note. Please restore the original copyright line (and keep the license header intact) and avoid embedding workflow-validation comments in SDK source files.
| # CI test: verifying chronus-verify workflow triggers for mgmt packages | |
| # | |
| # Copyright (c) Microsoft Corporation. All rights reserved. |
| - name: Install chronus | ||
| run: npm install -g @chronus/chronus | ||
|
|
||
| - name: Run chronus verify | ||
| run: chronus verify |
There was a problem hiding this comment.
Installing Chronus globally (npm install -g) is more fragile in CI (can fail due to npm global prefix permissions) and makes the executed Chronus version implicit. Prefer invoking Chronus via npx (optionally pinned to a specific version) so the workflow is reproducible and doesn’t depend on global PATH mutations.
| - name: Install chronus | |
| run: npm install -g @chronus/chronus | |
| - name: Run chronus verify | |
| run: chronus verify | |
| - name: Run chronus verify | |
| run: npx -y @chronus/chronus@latest verify |
|
@copilot can we have our gh workflow reference the doc on failure |
Co-authored-by: l0lawrence <100643745+l0lawrence@users.noreply.github.com>
Done in commit
Good question. The Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
mikeharder
left a comment
There was a problem hiding this comment.
pin version of chronus and all transitive deps
see https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/changelog_updates.md
Need to ping partner teams b4 merging