[TASK] Rename methods in RuleContainer#1521
Conversation
|
This could probably be split into a separate PR for each method rename if desired... Also there are a couple of comments I updated with alterations not entirely related to this PR, so I'll commit those first. |
oliverklee
left a comment
There was a problem hiding this comment.
I'd be fine with not splitting this up.
| */ | ||
| trait LegacyRuleContainerMethods | ||
| { | ||
| public function addRule(Declaration $declarationToAdd, ?Declaration $sibling = null): void |
There was a problem hiding this comment.
All these methods should get an @deprecated annotation stating since which version they are deprecated and in which version they will be removed (like in RuleContainer).
There was a problem hiding this comment.
I wondered about this, as I had marked the trait as @internal, but the methods are public and implemented in the classes using the trait, and this would be the only place to see the deprecation notice other than in the interface.
I've added the deprecation notices into the trait.
Should the trait nonetheless be marked as @internal?
There was a problem hiding this comment.
No, the trait should not be @internal. The reason is that IDEs mark usages of @internal methods and classes (so that the devs know that they should switch to something else), but probably not if only the trait it @internal.
There was a problem hiding this comment.
I've removed @internal and also added a description for the trait.
87e9ba5 to
2fa42c0
Compare
E.g. `addRule()` is changed to `addDeclaration()`, with `addRule()` deprecated and changed to a wrapper method to call on to `addDeclaration()`. `RuleContainer` itself should be renamed to `DeclarationContainer` as a separate change. The tests will also be updated as a separate change.
2fa42c0 to
6a3fec8
Compare
Also add a description for the trait.
The methods of the `RuleContainer` interface were renamed in #1521.
The methods of the `RuleContainer` interface were renamed in #1521.
E.g.
addRule()is changed toaddDeclaration(), withaddRule()deprecated and changed to a wrapper method to call on toaddDeclaration().RuleContaineritself should be renamed toDeclarationContaineras a separate change.The tests will also be updated as a separate change.