Copilot commented on code in PR #839:
URL: https://github.com/apache/iceberg-cpp/pull/839#discussion_r3629105320
##########
.github/workflows/zizmor.yml:
##########
@@ -23,8 +23,14 @@ on:
push:
branches:
- main
+ paths:
+ - '.github/**/*.yml'
+ - '.github/**/*.yaml'
pull_request:
types: [opened, synchronize, reopened, ready_for_review]
Review Comment:
This workflow is scoped for `pull_request` via `paths`, but the `push`
trigger to `main` remains unfiltered. The PR description says these validators
should run only for YAML changes under `.github`; to match that, add the same
`paths` filter under `on.push` (or clarify that push-to-main should always run
regardless of changed paths).
##########
.github/workflows/test.yml:
##########
@@ -23,8 +23,20 @@ on:
- main
tags:
- '**'
+ paths-ignore:
+ - '.devcontainer/**'
+ - '*.md'
+ - '**/*.md'
+ - 'dev/**'
+ - 'mkdocs/**'
pull_request:
types: [opened, synchronize, reopened, ready_for_review]
Review Comment:
`'*.md'` is redundant if `'**/*.md'` is present (as `'**/*.md'` also matches
root-level markdown files). Consider dropping `'*.md'` to reduce
duplication—same pattern appears in multiple workflows in this PR.
##########
.github/workflows/cpp-linter.yml:
##########
@@ -22,17 +22,27 @@ on:
branches:
- main
paths-ignore:
Review Comment:
The `push` trigger no longer has any `paths-ignore` filtering (it was
removed in this PR). This will cause the C++ linter to run on every push to
`main`, including changes that were previously excluded (e.g., `.github/**`,
`ci/**`, `cmake_modules/**`). If the PR’s intent is to reduce unnecessary CI
work, consider restoring an equivalent `paths-ignore` (or adding scoped
`paths`) under `on.push` as well.
##########
.github/workflows/test.yml:
##########
@@ -23,8 +23,20 @@ on:
- main
tags:
- '**'
+ paths-ignore:
+ - '.devcontainer/**'
Review Comment:
Only the `pull_request` trigger is being filtered; `push` is still
configured to run for all paths (`'**'`). If the goal is to skip CI work for
documentation/dev-only changes generally (including after merge), you’ll also
need to apply equivalent path filtering to the `push` trigger (or remove/adjust
`push` triggers where appropriate).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
