jeremykendall / php-domain-parser

Public Suffix List based domain parsing implemented in PHP
MIT License
1.16k stars 128 forks source link

ext-curl should be required on composer.json #252

Closed devnix closed 4 years ago

devnix commented 4 years ago

Introduction

This library sets ext-intl as a dependency to work, but does not check if ext-curl is installed as it seems to be. It throws an Error: Undefined constant 'CURLOPT_FAILONERROR' until I install the extension.

Proposal

Updating composer.json

I would add the requirement as a dependency on composer.json to make people aware of it.

nyamsprod commented 4 years ago

@devnix thanks for the PR submission while it is true that you need the ext-curl extension to use the shipped http-client, like written in the documentation, you do not require the extension to use the package as you could use any type of http-client which implements the package interface. For instance nothing is stopping an implementation from using PHP's file_get_contents to retrieve the data. That's why the ext-curl extension is in the suggest section of the composer.json file and not in the require one.

nyamsprod commented 4 years ago

@devnix sorry I accidently merge your PR while there's really no need to as explained So I reverted the merge but you will have to consider that your PR was rejected for the reason explained. Thanks for you tentative contribution and I hope that you will continue to contribute to this package in the future.