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

Improve debugging via Vardumper overriding #19308

Merged
merged 1 commit into from Jun 9, 2022
Merged

Improve debugging via Vardumper overriding #19308

merged 1 commit into from Jun 9, 2022

Conversation

ulcuber
Copy link
Contributor

@ulcuber ulcuber commented Jun 2, 2022

Description:

Fixes regression of #6890
Same as 6130e7e

Review

@Findus23
Copy link
Member

Findus23 commented Jun 2, 2022

Hi,

Can you maybe explain a bit more why this change is needed? The linked PR and commit are nearly a decade old and we are currently more in the process of removing upgradephp (#16650)

@ulcuber
Copy link
Contributor Author

ulcuber commented Jun 2, 2022

@Findus23 currently dump() does nothing because it is defined as empty function in libs/upgradephp/upgrade.php. That definition must come after original var-dumper autoloading (via require_once PIWIK_VENDOR_PATH . '/autoload.php';)

@ulcuber
Copy link
Contributor Author

ulcuber commented Jun 2, 2022

@Findus23 we could move it in other place but I'm not sure where it should be

@justinvelluppillai justinvelluppillai added the Needs Review PRs that need a code review label Jun 3, 2022
sgiehl
sgiehl approved these changes Jun 9, 2022
View reviewed changes
Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

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

Hm. I guess this library has never been used by anyone, as no one yet reported that it didn't work as expected.
Your fix makes it work again, so I will merge it. Thanks for that 👍

@sgiehl sgiehl added this to the 4.12.0 milestone Jun 9, 2022
@sgiehl sgiehl added Easier debugging For issues that make troubleshooting issues for developers easier. and removed Needs Review PRs that need a code review labels Jun 9, 2022
@sgiehl sgiehl merged commit a74aba9 into matomo-org:4.x-dev Jun 9, 2022
peterhashair pushed a commit that referenced this pull request Jun 16, 2022
@ulcuber
fixed: vardumper overriding (#19308)
612406a
@justinvelluppillai justinvelluppillai changed the title Vardumper overriding Improve debugging via Vardumper overriding Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sgiehl sgiehl sgiehl approved these changes

Assignees
No one assigned
Labels
Easier debugging For issues that make troubleshooting issues for developers easier.
Projects
None yet
Milestone
4.12.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ulcuber @Findus23 @sgiehl @justinvelluppillai