**ci(filters): update exclusion patterns for CI workflows**#35779
Conversation
- Adjusted `.github/filters.yaml` to include refined exclusion patterns under specific directories: - Added `dotCMS/src/main/webapp/WEB-INF/messages/!(Language*.properties)` to avoid excluding language property files. - Updated patterns to ensure that non-relevant changes in `src/main/webapp/html` and `WEB-INF/messages` directories are correctly filtered. - Updated `area_backend` filters to align with the modified path matching rules for better targeting of backend-specific changes. ref: #35778 Signed-off-by: erickgonzalez <erick.gonzalez@dotcms.com>
|
Claude finished @erickgonzalez's task in 2m 54s —— View job PR Review:
|
| Path | Line 17 match? | Line 18 match? | Net |
|---|---|---|---|
messages/Language_en.properties |
no (excluded by !(Language*.properties)) |
no (excluded by !(...messages/)) |
skip backend |
messages/unescape.sh |
yes | no | match |
messages/en_US/init.html |
yes | no | match |
dotCMS/src/main/java/Foo.java |
no | yes | match |
dotCMS/src/main/webapp/html/foo.jsp |
no | yes; also line 13 | match |
dotCMS/src/main/webapp/html/foo.css |
no | no (line 18 excludes html/, line 13 excludes css/js) | skip → goes to frontend ✓ |
Same logic reproduces in area_backend, with the (intentional?) difference that area_backend does not exclude html/. That asymmetry was pre-existing and not introduced here.
Things worth a second look
-
Picomatch extglob with slashes is the load-bearing assumption. Both
dotCMS/!(src/main/webapp/html/|src/main/webapp/WEB-INF/messages/)**and the originaldotCMS/!(src/main/webapp/html/)**rely on!(...)matching multi-segment substrings. Standard glob extglob is single-segment; picomatch happens to support slashes inside!(...)but it's not portable and not covered by your filter comments. Since the existing pattern presumably works in CI, extending it is reasonable — but this is not unit-tested anywhere. Recommend running a dry PR that touches onlyLanguage.propertiesand confirmingbackend/jvm_unit_test/area_backendall evaluate false before relying on it for cost savings. Without that, a silent regression here costs nothing visible and is hard to notice. -
Line 17 is redundant for the test-skip goal, but not for
area_backend. In thebackend:filter, line 18 already includes everything outside html/ and messages/. Line 17 only re-adds non-Language*files inside messages/, which means changes to (say)messages/unescape.shstill trigger backend tests. That's the intent — but it deserves a one-line comment in the YAML, otherwise the next person to touch this file will think the two lines contradict each other. -
area_backendchange is broader than the PR description says. The description mentions filter refinement for messages, but the diff also implicitly narrowsarea_backendfromdotCMS/**todotCMS/!(.../messages/)**. Net effect: PRs that only changeLanguage*.propertieswill no longer get the Area : Backend label at all. If labeling is used for routing/reviewers/release notes, confirm that's desired — a translation-only PR may legitimately want backend-team visibility. -
No test/example added. The file already documents (lines 78–93) that negation handling is fiddly. Adding a fragile new pattern without a verification step (even a comment block listing expected matches/non-matches) is the kind of thing that quietly breaks 3 months from now when someone refactors. Consider committing a small fixture or at minimum expanding the comment with "Language*.properties intentionally do not trigger backend CI — translation-only changes".
-
Minor: ordering. The two new lines under
backend:are added betweene2e/**anddotcms-integration/**, splitting the visual grouping (dotCMS-related patterns mixed with other modules). Cosmetic, but moving them adjacent todotCMS/src/main/webapp/html/...on line 13 would make the html/messages exclusion story read as a unit.
Nothing blocking
Pattern logic is sound under the picomatch-with-slashes assumption that the file already depends on. Suggest verifying with a translation-only test PR before merge and adding a comment explaining the messages/ rationale.
· branch: issue-35778-improvement-CI-tests
- Updated `.github/filters.yaml` to broaden the exclusion pattern for `WEB-INF/messages/`: - Changed `WEB-INF/messages/!(Language*.properties)` to `WEB-INF/messages/**/!(Language*.properties)` to include nested directories. - Ensures accurate filtering of localization files during CI runs. ref: #35778 Signed-off-by: erickgonzalez <erick.gonzalez@dotcms.com>
.github/filters.yamlto include refined exclusion patterns under specific directories:dotCMS/src/main/webapp/WEB-INF/messages/!(Language*.properties)to avoid excluding language property files.src/main/webapp/htmlandWEB-INF/messagesdirectories are correctly filtered.area_backendfilters to align with the modified path matching rules for better targeting of backend-specific changes.Fixes: #35778
This PR fixes: #35778