Skip to content

Conversation

@remicollet
Copy link
Member

  • ZipArchive::LENGTH_TO_END is always defined as 0
  • ZipArchive::LENGTH_UNCHECKED will be defined with libzip 1.10.1 planned for August

Already in pecl/zip, will be in version 1.22.2 planned to be released immediately after libzip

I think this is better to have in 8.3 before RC

@remicollet
Copy link
Member Author

See nih-at/libzip#397

* @var int
* @cvalue ZIP_LENGTH_TO_END
*/
public const LENGTH_TO_END = UNKNOWN;
Copy link
Member

Choose a reason for hiding this comment

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

you can now declare the constant types :) P.S.: Would you be OK to convert all ZipArchive class constants to typed?

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer to not, as same code is also used for older PHP versions, and don't want to much diff.

Copy link
Member Author

Choose a reason for hiding this comment

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

Btw, compat macro works, so done

Also in pecl pierrejoye/php_zip@f8abd19

/**
* @var int
*/
public const LENGTH_TO_END = 0;
Copy link
Member

Choose a reason for hiding this comment

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

please avoid declaring the same constant "twice" when possible as doing so may introduce problems to the generation of the manual AFAIR. E.g. You could conditionally declare ZIP_LENGTH_TO_END with the default value in the header file.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@remicollet remicollet requested a review from kocsismate July 28, 2023 11:58
@Girgias
Copy link
Member

Girgias commented Jul 28, 2023

LGTM

Copy link
Member

@kocsismate kocsismate left a comment

Choose a reason for hiding this comment

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

Only an upgrade note is needed, but LGTM!

@remicollet
Copy link
Member Author

Thanks for the review

Merged as

@remicollet remicollet closed this Jul 28, 2023
@remicollet remicollet deleted the issue-libzip-1.10.1 branch July 28, 2023 13:58
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.

3 participants