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

zf1s/zf1: require php extensions in dev only #136

Merged
merged 1 commit into from
Nov 8, 2022

Conversation

falkenhawk
Copy link
Member

@falkenhawk falkenhawk commented Oct 7, 2022

followup on #133, #128 (comment)

It is probably too restrictive to have them listed in require section in the root composer.json file.

Composer will complain that an extension is not installed, e.g. ext-ldap for Zend_Ldap, even if it's not needed i.e. a project is not using Zend_Ldap at all.

> composer require zf1s/zf1

  [InvalidArgumentException]                                                                                  
  Package zf1s/zf1 has requirements incompatible with your PHP version, PHP extensions and Composer version:  
    - zf1s/zf1 1.15.0 requires ext-ldap * but it is not present.    

Due to the nature of zf1, you may never need some of the extensions, as long as you do not use components which require them. But when you start using them, those components themselves should throw errors that required extensions are not installed.

Moved to require-dev to ensure they are installed when running tests on dev/CI.

I've considered adding them to suggest section but imo composer might spam too much with irrelevant information, when installing zf1s/zf1 package.

ref: #128 (comment)

@falkenhawk falkenhawk changed the title require php extensions in dev only zf1s/zf1: require php extensions in dev only Oct 7, 2022
Copy link
Contributor

@glensc glensc left a comment

Choose a reason for hiding this comment

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

require-dev is indeed better than suggests, as then the dependencies can be enforced in ci

Copy link
Contributor

@marcing marcing left a comment

Choose a reason for hiding this comment

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

I approve this change.

It is probably too restrictive to have them listed in `require` section in the root `composer.json` file.

Due to the nature of zf1, you may never need some of the extensions, as long as you do not use components which require them. But when you start using them, those components themselves should throw errors that required extensions are not installed.

Moved to `require-dev` to ensure they are installed when running tests on dev/CI.

I've considered adding them to `suggest` section but imo composer might spam too much with irrelevant information, when installing `zf1s/zf1` package.
@falkenhawk falkenhawk force-pushed the require-php-extensions-in-dev-only branch from 3bb41b5 to 8a1ab61 Compare November 8, 2022 12:37
@falkenhawk falkenhawk merged commit 6f8e98e into master Nov 8, 2022
@falkenhawk falkenhawk deleted the require-php-extensions-in-dev-only branch November 28, 2022 16:43
@falkenhawk falkenhawk mentioned this pull request Dec 12, 2022
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.

3 participants