[TASK] Rename RuleContainer to DeclarationList#1530
Conversation
4b6803f to
8ce7499
Compare
|
I forgot to add the If I'd pushed the changes as two separate commits (first rename, then add replacement with |
It contains property declarations not rules, and describing it as a 'list' rather than a 'container' seems more apt, as the declarations are ordered. Changes to use the new interface name in the source code and tests will be submitted as follow-up PRs.
8ce7499 to
0f080ff
Compare
|
|
||
| bootstrapFiles: | ||
| - %currentWorkingDirectory%/src/Rule/Rule.php | ||
| - %currentWorkingDirectory%/src/RuleSet/RuleContainer.php |
There was a problem hiding this comment.
We'll need to replace these additional bootstrap file as soon as possible (in a dedicated PR) with something that also works for consumers of our library. At the moment, PHPStan fails to find these aliases within Emogrifier, and other consumers will run into the same problem.
There was a problem hiding this comment.
We'll need to replace these additional bootstrap file
Oh. I don't know of a way of doing that.
The aliases allow us to make a minor release using the new class names, with the old class names deprecated. Then they will be dropped in the next major release.
I expected Stan to find the alias files; when I found it didn't, I discoved this means of telling it.
Is there a way of providing some PHPStan config for consumers? Or maybe a 'dev' Composer autoload setting could work?
There was a problem hiding this comment.
I think we can try these approaches (in this order until one solves the problem):
- Add a file
autoloading/class-aliases.php(or something similar), move all class aliases there, and add it toautoload.filesincomposer.json. - Try the TYPO3 class alias loader (in PHP-CSS-Parser, not in Emogrifier, so we know this works for consumers)
There was a problem hiding this comment.
🤞
I see there is a branch in Emogrifier that uses the dev branch of PHP-CSS-Parser, but not sure how to experimentally hack Composer's vendor directory. Though maybe if there is a way via Composer to avoid the PHPStan bootstrap settings within PHP-CSS-Parser itself, that may solve the problem for consumers.
There was a problem hiding this comment.
We can create a PR (or a branch) here in PHP-CSS-Parser and then create a branch in Emogrifier that uses that branch (similar to the PR that uses PHP-CSS-Parser main) and then either run the tests locally or on CI for a PR.
There was a problem hiding this comment.
I've just entered #1532 for this and will give it a try.
There was a problem hiding this comment.
@JakeQZ This is how I tested this in Emogrifier without having to push to GitHub first: MyIntervals/emogrifier#1570
The interface was renamed from `RuleContainer` in #1530. Also accordingly rename the trait that provides test methods for implementing classes.
The interface was renamed from `RuleContainer` in #1530. Also accordingly rename the trait that provides test methods for implementing classes.
The interface was renamed from `RuleContainer` in #1530. Also accordingly rename the trait that provides test methods for implementing classes.
The interface was renamed from `RuleContainer` in #1530. Also accordingly rename the trait that provides test methods for implementing classes.
The interface was renamed from `RuleContainer` in #1530. Also accordingly rename the trait that provides test methods for implementing classes.
It contains property declarations not rules, and describing it as a 'list' rather than a 'container' seems more apt, as the declarations are ordered.
Changes to use the new interface name in the source code and tests will be submitted as follow-up PRs.