Skip to content

Autotools: Sync CS in gd extension #15364

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

Merged
merged 1 commit into from
Aug 12, 2024
Merged

Autotools: Sync CS in gd extension #15364

merged 1 commit into from
Aug 12, 2024

Conversation

petk
Copy link
Member

@petk petk commented Aug 12, 2024

  • Obsolete PHP_* variables checks removed (there was once the 'pdf' extension bundled in PHP that also had the same --with-*-dir configure options (3be17e3). When combined with the gd extension, options need to be executed conditionally; first one won), this is no longer relevant neither recommended practice to duplicate configure options inside the php-src context. Ideally, all configure options should be prefixed with an extension namespace --with-<extension-name>-<option> to be unique.
  • AS_* macros used

- Obsolete PHP_* variables checks removed (there was once the 'pdf'
  extension bundled in PHP that also had the same --with-*-dir configure
  options (3be17e3). When combined with
  the gd extension, options need to be executed conditionally; first one
  won), this is no longer relevant neither recommended practice to
  duplicate configure options inside the php-src context. Ideally,
  all configure options should be prefixed with an extension namespace
  --with-<extension-name>-<option> to be unique.
- AS_* macros used
Copy link
Member

@devnexen devnexen left a comment

Choose a reason for hiding this comment

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

LGTM

@petk petk merged commit 8c4f019 into php:master Aug 12, 2024
10 checks passed
@petk petk deleted the patch-m4-cs-gd branch August 12, 2024 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants