Skip to content

Conversation

asgrim
Copy link

@asgrim asgrim commented Jan 20, 2025

Originally in #12 this PR:

  • fixes the Windows build failures
  • makes a couple of suggested improvements to PIE's composer.json

"php": ">=5.4"
},
"php-ext": {
"extension-name": "timezonedb",
Copy link
Author

Choose a reason for hiding this comment

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

Extension name is not strictly required, since the package name is already correct, but it may help to be explicit.

"configure-options": []
"configure-options": [
{
"name": "enable-timezonedb",
Copy link
Author

Choose a reason for hiding this comment

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

This --enable-timezonedb option is defined in config.m4 so it should be possible to provide this in the configure-options when using PIE

Copy link
Member

Choose a reason for hiding this comment

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

But it is pointless. Using it is the default, doing --disable-timezonedb makes the extension not work.

"configure-options": []
"configure-options": [
{
"name": "enable-timezonedb",
Copy link
Member

Choose a reason for hiding this comment

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

But it is pointless. Using it is the default, doing --disable-timezonedb makes the extension not work.

- { os: windows-2019, php: "8.1" }
- { os: windows-2019, php: "8.0" }
- { os: windows-2022, php: "7.4" }
- { os: windows-2022, php: "7.3" }
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be better to just spell out all the right combinations instead of the wrong ones?

derickr added a commit that referenced this pull request Sep 25, 2025
@derickr
Copy link
Member

derickr commented Sep 25, 2025

I've picked the relevant bits into #18 , thanks!

@derickr derickr closed this Sep 25, 2025
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.

2 participants