-
Notifications
You must be signed in to change notification settings - Fork 0
🩹 [Patch] Workflow improvements #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Create Release.yml workflow to handle pull request events on the main branch. - Implement concurrency control and set permissions for creating releases and commenting on PRs. - Add steps to checkout code and utilize PSModule/Auto-Release action for managing releases. feat: Introduce JSON schema for PSModule settings - Add Settings.schema.json to define the configuration schema for PSModule GitHub Action settings. - Include properties for module name, test configurations, build settings, publish options, linter settings, and runtime execution flags. feat: Implement main script for PSModule GitHub Action - Create main.ps1 script to handle input parameters, import settings, validate against schema, and calculate job run conditions. - Add logic for determining if tests should run based on pull request changes and settings. - Implement functionality to manage test suites and handle important file changes for triggering builds and releases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements workflow optimization by introducing conditional execution of CI/CD stages based on whether a pull request modifies important files (source code, examples, documentation, or workflow configuration). The main script now fetches changed files from the GitHub API and skips build/test/publish stages for PRs that only touch non-critical files, reducing unnecessary CI resource usage.
Changes:
- Added file change detection logic in
src/main.ps1to identify PRs with important changes and conditionally run build/test stages - Updated workflow trigger conditions and action versions across multiple workflow files
- Changed Release workflow from
pull_request_targettopull_requestevent with path filters
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/main.ps1 | Added important file change detection via GitHub API, conditional execution logic for build/test stages, and PR commenting for skipped stages |
| src/Settings.schema.json | Added HasImportantChanges boolean property to schema |
| action.yml | Updated GitHub-Script action to v1.7.10 and corrected script path from scripts/main.ps1 to src/main.ps1 |
| .github/workflows/Release.yml | Renamed from Auto-Release to Release, changed trigger from pull_request_target to pull_request, added path filters, updated to Release-GHRepository v2.0.1, and updated checkout action to v6.0.2 |
| .github/workflows/Linter.yml | Updated checkout action to v6.0.2 |
| .github/workflows/Action-Test.yml | Updated checkout action to v6.0.2 |
| .github/linters/.jscpd.json | Removed JSCPD configuration file (JSCPD validation is disabled in Linter.yml) |
Comments suppressed due to low confidence (8)
src/main.ps1:525
- The comment states "For merged PRs, workflow_dispatch, schedule - $hasImportantChanges is already true", but this is incorrect. For merged PRs, the code still checks which files changed (lines 240-306), and if no important files changed,
$hasImportantChangeswill befalse. Only for non-PR events (workflow_dispatch, schedule) is it automatically set totrue(lines 307-311).
This means for merged PRs without important changes, $shouldRunBuildTest will be false, and build/test/docs stages won't run, which appears to be the intended behavior based on the PR description. However, the comment is misleading and should be corrected.
src/main.ps1:222
- The variable name
$isOpenOrUpdatedPRnow includes the 'labeled' and 'unlabeled' actions. While this makes sense for triggering linting when labels change, the name "Updated" typically implies code changes (synchronize) rather than metadata changes (labels). Consider renaming to something like$isOpenOrModifiedPRor$shouldProcessPRfor clarity, or add a comment explaining that "updated" includes label changes.
src/main.ps1:304 - The condition
$isOpenOrUpdatedPRincludes the 'synchronize' action, which means a new comment will be posted every time the PR is updated with new commits. This could result in multiple duplicate comments on the same PR if it's updated several times without important file changes.
Consider tracking whether a comment has already been posted, or only post the comment once on 'opened' action. Alternatively, update an existing comment rather than creating new ones.
.github/workflows/Release.yml:6
- Changing from
pull_request_targettopull_requestchanges the security context of the workflow. Withpull_request, the workflow runs with the permissions and code from the PR branch, which could be from a fork. This is generally safer for most operations, but means the workflow will run the code from the PR author's branch rather than the base branch.
Since this workflow now has contents: write and pull-requests: write permissions, and the Release action could potentially publish releases, ensure that the PSModule/Release-GHRepository action has appropriate security checks to prevent malicious PRs from creating unauthorized releases. This is particularly important since the workflow now runs untrusted code from PR branches.
.github/workflows/Release.yml:17
- The workflow-level
pathsfilter only includes 'action.yml' and 'src/', but the script logic in main.ps1 (lines 255-260) also considers 'examples/', 'README.md', and '.github/workflows/Process-PSModule.yml' as important files. This creates an inconsistency where:
- If only 'examples/**' or 'README.md' changes, the Release workflow won't run at all (filtered out by paths)
- But if it did run, main.ps1 would consider these as important changes
Either the workflow paths filter should match the patterns in main.ps1, or this intentional filtering should be documented. The current mismatch could lead to confusion where some "important" changes don't trigger the workflow.
src/main.ps1:280
- The condition for adding the "no important changes" comment uses
$isOpenOrUpdatedPR, which now includes 'unlabeled' (line 222). This means removing a label from a PR could trigger the comment about missing important file changes, even though the lack of important changes is not new information - no files were changed by the unlabel action.
Consider using a more specific condition that only triggers the comment when file changes actually occur (opened, reopened, synchronize) rather than label changes.
src/main.ps1:249
- The GitHub API endpoint
/repos/{owner}/{repo}/pulls/{pull_number}/filesis paginated and returns a maximum of 30 files per page by default (up to 100 with per_page parameter). For PRs with more than 30 (or 100) changed files, this code will only check the first page of results, potentially missing important file changes in larger PRs.
Consider adding pagination handling to ensure all changed files are checked, or at minimum add a warning when the result count suggests pagination might be needed.
src/main.ps1:249
- The API call to fetch changed files lacks error handling. If the call fails (due to network issues, rate limiting, authentication problems, etc.),
$changedFileswill be null or empty, and the script will incorrectly conclude that no important files have changed, potentially skipping necessary build/test stages.
Consider wrapping this in a try-catch block and either failing explicitly with a clear error message, or defaulting to $hasImportantChanges = $true when the API call fails to ensure builds run in case of transient API issues.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This release introduces several improvements to the GitHub Actions workflows and the main PowerShell script for the repository. The main focus is to optimize CI/CD runs by detecting whether a pull request includes changes to important files (such as source code, examples, documentation, or workflow configurations). Build, test, and publish stages are now conditionally executed based on this detection, which helps save CI resources and provides clear feedback to contributors. Additionally, the workflows and dependencies have been updated and refactored for clarity and maintainability.
Workflow optimization and conditional execution:
src/main.ps1to detect if a PR changes important files (src/**,examples/**,README.md,.github/workflows/Process-PSModule.yml). If not, build/test stages are skipped, and a comment is added to the PR explaining why. This is also reflected in the newHasImportantChangesproperty in both the script andSettings.schema.json.Workflow and dependency updates:
actions/checkoutandPSModule/GitHub-Scriptactions to their latest versions in all workflow files andaction.ymlfor improved security and features.action.ymlto usesrc/main.ps1instead of the oldscripts/main.ps1location.Auto-ReleasetoRelease, updated its triggering conditions, and switched to the newPSModule/Release-GHRepositoryaction for releases.Cleanup and configuration changes:
.github/linters/.jscpd.jsonconfiguration file, possibly as part of linter or duplication check cleanup.scripts/Settings.schema.jsontosrc/Settings.schema.json).These changes collectively make the CI/CD workflows more efficient, transparent, and easier to maintain.