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

Jump to bottom

Fixes HTML_QuickForm2_ContainerIterator for PHP8.1 #18384

Merged
merged 2 commits into from Dec 2, 2021
Merged

Fixes HTML_QuickForm2_ContainerIterator for PHP8.1 #18384

merged 2 commits into from Dec 2, 2021

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Nov 26, 2021

Description:

missed that in #18309

fixes #18374

Review

@sgiehl sgiehl added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Nov 26, 2021
@sgiehl sgiehl added this to the 4.6.0 milestone Nov 26, 2021
@sgiehl sgiehl requested a review from tsteur November 26, 2021 08:35
@sgiehl sgiehl linked an issue Nov 26, 2021 that may be closed by this pull request
[PHP8.1] Deprecated return type of HTML_QuickForm2_ContainerIterator #18374
Closed
@MichaIng
Copy link
Contributor

Are those probably required here as well? https://github.com/matomo-org/matomo/blob/c4431ba/libs/HTML/QuickForm2/Element/Select.php#L63-L69

@sgiehl
Copy link
Member Author

sgiehl commented Nov 26, 2021

@MichaIng thanks for spotting that. I've pushed a fix.

@justinvelluppillai justinvelluppillai modified the milestones: 4.6.0, 4.7.0 Nov 30, 2021
Base automatically changed from next_release to 4.x-dev November 30, 2021 20:16
@sgiehl sgiehl requested review from justinvelluppillai and removed request for tsteur December 2, 2021 08:56
@justinvelluppillai
Copy link
Contributor

@sgiehl were you able to reproduce this issue? I have been unsuccessful, even calling $form->getElementsByName() directly doesn't give any error for me in PHP8.1.

The changes look fine but it'd be good to see them fix an actual issue that I can repro 😝

@sgiehl
Copy link
Member Author

sgiehl commented Dec 2, 2021

Actually I couldn't. Those I could reproduce were already fixed in another PR.

justinvelluppillai
justinvelluppillai approved these changes Dec 2, 2021
View reviewed changes
Copy link
Contributor

@justinvelluppillai justinvelluppillai left a comment

Choose a reason for hiding this comment

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

Seems like this will fix the reported issue the same as the other fixes you made.

@justinvelluppillai justinvelluppillai merged commit c7ae86c into 4.x-dev Dec 2, 2021
@justinvelluppillai justinvelluppillai deleted the m-18374 branch December 2, 2021 20:21
@MichaIng
Copy link
Contributor

I'm not sure how it could have happened, but Matomo v4.6.2 has now been released without this commit. Ah, I see now the v4.7.0 milestone, just wondering why since this is a trivial but effective fix without any backwards compatibility issues 🤔. However, I'll re-apply it again manually.

@sgiehl sgiehl mentioned this pull request Jan 31, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justinvelluppillai justinvelluppillai justinvelluppillai approved these changes

Assignees
No one assigned
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
4.7.0
Development

Successfully merging this pull request may close these issues.

[PHP8.1] Deprecated return type of HTML_QuickForm2_ContainerIterator
3 participants
@sgiehl @MichaIng @justinvelluppillai