Conversation
Triggers on push to main when yaml schema files change, and on manual dispatch. Regenerates Python datamodel, enums, and docs, then opens a PR with the changes. Closes any previous unclosed regen PRs before creating a new one. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Claude finished @cmungall's task —— View job PR Review Complete ✅Todo List
Review FeedbackThis is a well-implemented GitHub Actions workflow with good security practices and clear logic. Here's my detailed analysis: ✅ StrengthsSecurity Best Practices:
Code Quality:
Logic & Error Handling:
🔧 Minor Suggestions
🎯 Performance Considerations
|
There was a problem hiding this comment.
Pull request overview
This PR adds a GitHub Actions workflow to automatically regenerate derived files (Python datamodel, rich enums, and documentation) when schema YAML files are modified. The workflow triggers on pushes to main affecting schema files or via manual dispatch, runs the regeneration commands, and creates a PR with the changes if any files were updated.
Changes:
- New automated workflow that runs
just gen-projectandjust gen-docon schema changes - Automatically closes previous regeneration PRs when superseded
- Creates labeled PRs for review of regenerated artifacts
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Install uv | ||
| uses: astral-sh/setup-uv@v6 | ||
| with: | ||
| enable-cache: true | ||
| cache-dependency-glob: "uv.lock" | ||
|
|
||
| - name: Set up Python | ||
| uses: actions/setup-python@v5 |
There was a problem hiding this comment.
Action versions should be pinned to specific versions for reproducibility and security. The repository uses pinned versions consistently in other workflows (e.g., actions/checkout@v4.2.2, astral-sh/setup-uv@v6.4.3, actions/setup-python@v5.6.0). Update these action references to match the versions used in main.yaml and deploy-docs.yaml.
| uses: actions/checkout@v4 | |
| with: | |
| fetch-depth: 0 | |
| - name: Install uv | |
| uses: astral-sh/setup-uv@v6 | |
| with: | |
| enable-cache: true | |
| cache-dependency-glob: "uv.lock" | |
| - name: Set up Python | |
| uses: actions/setup-python@v5 | |
| uses: actions/checkout@v4.2.2 | |
| with: | |
| fetch-depth: 0 | |
| - name: Install uv | |
| uses: astral-sh/setup-uv@v6.4.3 | |
| with: | |
| enable-cache: true | |
| cache-dependency-glob: "uv.lock" | |
| - name: Set up Python | |
| uses: actions/setup-python@v5.6.0 |
| on: # yamllint disable-line rule:truthy | ||
| push: | ||
| branches: [main] | ||
| paths: | ||
| - "src/valuesets/schema/**/*.yaml" | ||
| - "src/valuesets/schema/*.yaml" | ||
| workflow_dispatch: |
There was a problem hiding this comment.
The workflow lacks concurrency control, which could lead to race conditions if multiple schema changes are pushed to main in quick succession. Multiple workflow runs could attempt to create PRs simultaneously, or a second run could start while the first is still creating its PR. Add a concurrency group to ensure only one regeneration workflow runs at a time and cancel any in-progress runs when a new one starts.
| run: uv sync --group dev --no-progress | ||
|
|
||
| - name: Regenerate derived files | ||
| run: | |
There was a problem hiding this comment.
The workflow doesn't verify that the regeneration commands succeeded before attempting to create a PR. If just gen-project or just gen-doc fails, the workflow will continue and potentially create a PR with incomplete or incorrect changes. Consider adding error handling or checks to ensure the regeneration completed successfully before proceeding to create the PR.
| run: | | |
| run: | | |
| set -euo pipefail |
| run: uv tool install rust-just | ||
|
|
||
| - name: Install dependencies | ||
| run: uv sync --group dev --no-progress |
There was a problem hiding this comment.
The workflow uses 'uv sync --group dev' while other workflows in this repository use 'uv sync --dev'. Since the pyproject.toml defines dev dependencies using dependency-groups, '--group dev' is technically correct. However, for consistency with existing workflows (main.yaml:52, deploy-docs.yaml:53), consider using '--dev' instead unless there's a specific reason to use the newer '--group' syntax.
| run: uv sync --group dev --no-progress | |
| run: uv sync --dev --no-progress |
| --body "$(cat <<'EOF' | ||
| Automated regeneration of derived files after schema changes. | ||
|
|
||
| Updated by running: | ||
| ``` | ||
| just gen-project | ||
| just gen-doc | ||
| ``` | ||
|
|
||
| Please review the diffs before merging. | ||
| EOF | ||
| )" |
There was a problem hiding this comment.
The PR body has inconsistent indentation which may result in incorrectly formatted markdown. The heredoc content (lines 94-102) is indented with leading spaces while the opening 'EOF' delimiter (line 93) has no indentation. This could cause the PR description to have unexpected leading whitespace. Consider aligning the heredoc content to start at column 0 or adjusting the indentation to be consistent.
| uses: astral-sh/setup-uv@v6 | ||
| with: | ||
| enable-cache: true | ||
| cache-dependency-glob: "uv.lock" |
There was a problem hiding this comment.
The astral-sh/setup-uv action should include the python-version parameter for consistency with other workflows in this repository. In deploy-docs.yaml:36-38, the python-version is explicitly set on the setup-uv action. While the separate setup-python step will still configure Python correctly, setting it on both actions ensures consistency and clarity.
| cache-dependency-glob: "uv.lock" | |
| cache-dependency-glob: "uv.lock" | |
| python-version: "3.13" |
Summary
Adds a new GitHub Actions workflow (
.github/workflows/regen-derived.yaml) that regenerates derived files when schema YAML files change.Triggers:
mainwhen anysrc/valuesets/schema/**/*.yamlfile changesworkflow_dispatch)What it does:
just gen-projectandjust gen-docto regenerate derived Python datamodel, rich enums, and documentationautomated-regenThis replaces the need to manually run
just siteand commit derived file changes after schema edits.Test plan
🤖 Generated with Claude Code