Skip to content

Fix docs generation command #7670

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.

Sign up for GitHub

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

Jump to bottom
Merged
westonruter merged 4 commits into from
Nov 3, 2023
Merged

Fix docs generation command #7670

westonruter merged 4 commits into from
Nov 3, 2023

Conversation

@thelovekesh
Copy link
Collaborator

@thelovekesh thelovekesh commented Nov 3, 2023

Summary

  • Fix the issue due to the PHP patch version in version compares.
  • Fix code issues that are causing PHP parser warnings.

Checklist

  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2023 •
edited
Loading

Plugin builds for 34bed2f are ready 🛎️!

Checksums 
# Development build checksums
2c7c7265c5d94df4189aceae3d5819266b52e9f40fa9f8d8ba061844b4045eb6 *amp.zip

# Production build checksums
9509d19f94f65823523bc7bf3a5e596106610399cd0cdeaa17d709e1cbc8aeae *amp.zip

Warning

These builds are for testing purposes only and should not be used in production.

@thelovekesh
Copy link
Collaborator Author

thelovekesh commented Nov 3, 2023

A few warnings are coming up while docs generation from the PHP parser.

[03-Nov-2023 14:45:07 UTC] PHP Deprecated:  Invalid characters passed for attempted conversion, these have been ignored in /var/www/html/wp-content/plugins/amp/vendor/nikic/php-parser/lib/PhpParser/Node/Scalar/String_.php on line 93
Deprecated: Invalid characters passed for attempted conversion, these have been ignored in /var/www/html/wp-content/plugins/amp/vendor/nikic/php-parser/lib/PhpParser/Node/Scalar/String_.php on line 93

These are specifically due to:

x0B in

amp-wp/includes/sanitizers/class-amp-base-sanitizer.php

Line 880 in 66b8668

$style_string = trim( $style_string, " \t\n\r\0\x0B;" );

xEF, xBB and xBF in

amp-wp/includes/validation/class-amp-validation-manager.php

Line 2344 in 66b8668

while ( "\xEF\xBB\xBF" === substr( $response, 0, 3 ) ) {

This can also be due to an old PHP parser or can be some legitimate warnings.

westonruter
westonruter reviewed Nov 3, 2023
View reviewed changes
@thelovekesh thelovekesh force-pushed the fix/docs-generation-command branch from f2651b8 to e9a7040 Compare November 3, 2023 17:04
westonruter
westonruter reviewed Nov 3, 2023
View reviewed changes
.github/dependabot.yml
Comment on lines 16 to 18
ignore:
- dependency-name: phpdocumentor/reflection
Copy link
Member

@westonruter westonruter Nov 3, 2023

Choose a reason for hiding this comment

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

Could you add a comment to this explaining why it is ignored?

Copy link
Collaborator Author

@thelovekesh thelovekesh Nov 3, 2023

Choose a reason for hiding this comment

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

Done in 34bed2f.

@thelovekesh thelovekesh force-pushed the fix/docs-generation-command branch from 7d6542f to 34bed2f Compare November 3, 2023 17:21
@westonruter westonruter added this to the v2.5.0 milestone Nov 3, 2023
@westonruter westonruter merged commit f7026e8 into develop Nov 3, 2023
@westonruter westonruter deleted the fix/docs-generation-command branch November 3, 2023 17:39
@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@westonruter westonruter westonruter approved these changes

Assignees

No one assigned

Labels

Changelogged Whether the issue/PR has been added to release notes.

Projects

None yet

Milestone

v2.5.0

Development

Successfully merging this pull request may close these issues.

3 participants

@thelovekesh @westonruter