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

Remove jwage/purl dependency #18

Merged
merged 1 commit into from
Oct 22, 2018
Merged

Remove jwage/purl dependency #18

merged 1 commit into from
Oct 22, 2018

Conversation

swissspidy
Copy link
Contributor

This is an alternative to #16.

Since the package was only used to replace the host, wp_parse_url() should work just as fine.

If the PHP requirement would be increased to PHP 5.4+, parse_url() could be used directly.

@swissspidy
Copy link
Contributor Author

@swalkinshaw Wanna review? 🙂

@swalkinshaw
Copy link
Member

We already have "php": ">=5.4.0". Is the = the issue here?

@swissspidy
Copy link
Contributor Author

Kinda. The thing is that parse_url() behavior was changed in PHP 5.4.7. So if you're running PHP 5.4-5.4.6, then wp_parse_url() is the safer bet.

But even if you're running a newer version of PHP it doesn't really hurt to use wp_parse_url().

@swalkinshaw
Copy link
Member

This is a WP plugin after all so wp_parse_url is fine 👍

@swalkinshaw swalkinshaw merged commit 5333b67 into roots:master Oct 22, 2018
@swalkinshaw swalkinshaw mentioned this pull request Oct 22, 2018
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.

None yet

2 participants