Conversation
bnbong
added
bug
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a36df24c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - changed-files: | ||
| - any-glob-to-any-file: | ||
| - 'src/**' | ||
| - all-globs-to-all-files: | ||
| - '!src/fastapi_fastkit/fastapi_project_template/**' |
There was a problem hiding this comment.
all wrapper for enhancement matcher
In actions/labeler@v5, a match object without all defaults to any, and the glob-combination entries under changed-files are OR-ed. That means all-globs-to-all-files: '!src/fastapi_fastkit/fastapi_project_template/**' can match PRs that never touch src/ (for example docs-only changes), so enhancement will be applied far too broadly. Wrapping these changed-files conditions in all is required to enforce "must touch src and must not be template".
Useful? React with 👍 / 👎.
| - changed-files: | ||
| - any-glob-to-any-file: | ||
| - 'src/fastapi_fastkit/fastapi_project_template/**' | ||
| - all-globs-to-all-files: | ||
| - '!src/fastapi_fastkit/fastapi_project_template/README.md' |
There was a problem hiding this comment.
all wrapper for template matcher
This rule has the same semantic problem: without a top-level all, the matcher defaults to any, so all-globs-to-all-files: '!src/fastapi_fastkit/fastapi_project_template/README.md' matches most PRs by itself (any PR where every changed file is not that exact README). As a result, the template label will still be attached to unrelated PRs instead of only template-path changes excluding the README case.
Useful? React with 👍 / 👎.
1 participant
Requesting Merging
Description
fixing PR labeler
Type of Change
Test Environment
N/A
Major Changes
Screenshots (optional)
N/A
Etc
N/A