Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creation of dynamic property is deprecated #3

Merged
merged 1 commit into from
Sep 3, 2022

Conversation

bytestream
Copy link

Creation of dynamic property SimpleCallSchedule::$expected_args is deprecated

Creation of dynamic property SimpleCallSchedule::$expected_args is deprecated
@TimWolla
Copy link

TimWolla commented Sep 2, 2022

When defining properties I also verify how the property is actually used to make the best decision. In this specific case it appears that the property is never read from, which indicates that the better fix might be removing the assignment. However because of the failing test suite I was not able to properly verify this.

@bytestream
Copy link
Author

bytestream commented Sep 2, 2022

@TimWolla I agree with what you're saying. However, as you've alluded to, without tests and anyone with knowledge of simpletest internals such changes are just asking for trouble. I think it's better to just maintain how things worked before. A dynamic property was previously declared public, so public $foo is the way to go.

Based on other classes private $expected_args; would probably also work, but again, it's difficult to know whether that dynamic property is used elsewhere.

@ezyang ezyang merged commit 2819641 into ezyang:master Sep 3, 2022
@bytestream bytestream deleted the patch-1 branch September 3, 2022 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants