Merge rule collections instead of replacing them - #2
Merged
Merged
Conversation
added 3 commits
September 24, 2026 14:04
RuleAggregate::addCollections() array_merged two arrays keyed by registrable domain. String keys are overwritten rather than appended, so when a second call carried rules for a domain an earlier call had already contributed to, the earlier collection was dropped entirely. This is reachable through createFromFiles(), which calls addCollections() once per file: any domain listed in more than one file kept only the rules of the file read last. It also made exceptions unreliable, since an @@ rule in one file would silently replace the blockers of another instead of overriding them. Add the incoming rules to the existing collection instead. Routing them through RuleCollection::addRule() keeps exceptions and blockers in their own buckets, so getRulesToApplyForDomain() still returns exceptions first. Also ignore the caches and the coverage report the test tooling writes.
The matrix only covered 8.1 and 8.2, while consumers of this package already run 8.5. Add 8.3, 8.4 and 8.5. The suite and php-cs-fixer were both verified against 8.4 and 8.5 before widening the matrix. Publish the coverage report from a single leg. Every leg produces the same report and uploads it under the same artifact name, so five of them would race where two already did. The style check was already failing on master, over an array indentation and a @PARAM naming an argument that does not exist.
upload-pages-artifact@v1 calls upload-artifact@v3 internally, which GitHub now fails outright, so every matrix job errored before running anything. Bump it to v3, which uses upload-artifact@v4, and deploy-pages to v4 to match on the download side. configure-pages goes to v5 for the same reason.
willaix
had a problem deploying
to
github-pages
September 24, 2026 13:22 — with 
GitHub Actions
Failure
GitHub Actions
Failure
The github-pages environment is protected to master, so the deploy job was rejected on every pull request with "Branch refs/pull/N/merge is not allowed to deploy to github-pages". The build job still uploads the artifact on pull requests, so that step stays covered before a change reaches master.
willaix
force-pushed
the
fix/collections-merge
branch
from
September 24, 2026 13:31
f76d24b to
f22cb2b
Compare
giuseppe-arcuti
approved these changes
Sep 24, 2026
mathroc
approved these changes
Sep 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RuleAggregate::addCollections() array_merged two arrays keyed by registrable domain. String keys are overwritten rather than appended, so when a second call carried rules for a domain an earlier call had already contributed to, the earlier collection was dropped entirely.
This is reachable through createFromFiles(), which calls addCollections() once per file: any domain listed in more than one file kept only the rules of the file read last. It also made exceptions unreliable, since an @@ rule in one file would silently replace the blockers of another instead of overriding them.
Add the incoming rules to the existing collection instead. Routing them through RuleCollection::addRule() keeps exceptions and blockers in their own buckets, so getRulesToApplyForDomain() still returns exceptions first.