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.

Sign up for GitHub

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

Jump to bottom

Fix getChildren must be compatible with SimpleXMLElement with PHP 8 #1613

Merged
merged 2 commits into from
May 23, 2021
Merged

Fix getChildren must be compatible with SimpleXMLElement with PHP 8 #1613

merged 2 commits into from
May 23, 2021

Conversation

colinmollenhour
Copy link
Member

…th SimpleXMLElement with PHP 8)

See #1603

@github-actions github-actions bot added the Component: Api PageRelates to Mage_Api label May 14, 2021
@colinmollenhour colinmollenhour mentioned this pull request May 14, 2021
Closed
3 tasks
@Flyingmana
Copy link
Contributor

I think it would still be good to keep the

public function getChildren($source=null)

but throwing a deprecation error, if the parameter is set to avoid surprises for changed behavior in case anyone was using it.
And if its not set, call the parent method, so its still exposed for usage.

@colinmollenhour
Copy link
Member Author

Added back the method with deprecated notice.

kiatng
kiatng approved these changes May 18, 2021
View reviewed changes
Copy link
Contributor

@kiatng kiatng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer this over PR #1603.

@Flyingmana Flyingmana added the PHP 8 Related to PHP8 label May 23, 2021
@Flyingmana Flyingmana changed the title Alternative solution for #1603 (Fix getChildren must be compatible wi… Fix getChildren must be compatible with SimpleXMLElement with PHP 8 May 23, 2021
@Flyingmana Flyingmana merged commit 70248fc into OpenMage:1.9.4.x May 23, 2021
@github-actions
Copy link
Contributor

Unit Test Results

1 files  ±0  1 suites  ±0   0s ⏱️ ±0s
0 tests ±0  0 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
6 runs  ±0  4 ✔️ ±0  2 💤 ±0  0 ❌ ±0 

Results for commit 70248fc. ± Comparison against base commit 434e403.

@Flyingmana Flyingmana mentioned this pull request May 23, 2021
Closed
@fballiano fballiano mentioned this pull request May 17, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Flyingmana Flyingmana Flyingmana approved these changes

@kiatng kiatng kiatng approved these changes

Assignees
No one assigned
Labels
Component: Api PageRelates to Mage_Api PHP 8 Related to PHP8
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@colinmollenhour @Flyingmana @kiatng