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

Improve url detection in mod pagespeed check #19595

Merged
merged 2 commits into from Aug 9, 2022
Merged

Improve url detection in mod pagespeed check #19595

merged 2 commits into from Aug 9, 2022

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Aug 3, 2022

Description:

fixes #12024

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 Aug 3, 2022
@sgiehl sgiehl added this to the 4.12.0 milestone Aug 3, 2022
Copy link
Contributor

@bx80 bx80 left a comment

Choose a reason for hiding this comment

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

This fixes the unknown host error for me 👍

@bx80 bx80 removed the Needs Review PRs that need a code review label Aug 5, 2022
@justinvelluppillai
Copy link
Contributor

@bx80 generally you can merge PRs on approval unless there's a reason to leave it for the author

@sgiehl
Copy link
Member Author

sgiehl commented Aug 5, 2022

Seems the installation was broken due to this change. Common::getPiwikUrl uses the database, which is not available during install. Added a fallback to the method used before the change...

@sgiehl sgiehl added the Needs Review PRs that need a code review label Aug 5, 2022
@sgiehl sgiehl requested a review from bx80 August 9, 2022 09:46
Copy link
Contributor

@bx80 bx80 left a comment

Choose a reason for hiding this comment

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

Tested successfully both from the command line and as part of installation when no database is available. 👍

@bx80 bx80 merged commit a8dec66 into 4.x-dev Aug 9, 2022
@bx80 bx80 deleted the m12024 branch August 9, 2022 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

Improve URL detection in mod_pagespeed check
3 participants