Navigation Menu

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

Syntax error in amp-translations.php can block deployments #1416

Closed
westonruter opened this issue Sep 10, 2018 · 2 comments · Fixed by #1427
Closed

Syntax error in amp-translations.php can block deployments #1416

westonruter opened this issue Sep 10, 2018 · 2 comments · Fixed by #1427
Assignees
Milestone

Comments

@westonruter
Copy link
Member

The pot-to-php script is used to generate an amp-translations.php file for WordPress.org to use for translations:

https://github.com/Automattic/amp-wp/blob/b3e1b4d07dda70c5070d30f7426f849eb3a39823/package.json#L34

However, the amp-translations.php file has a PHP syntax error with a spurious comma on line 6:

<?php
/* THIS IS A GENERATED FILE. DO NOT EDIT DIRECTLY. */
$generated_i18n_strings = array(
	/* translators: Copyright (C) 2018 WordPress.com VIP, XWP, Google, and contributors
This file is distributed under the same license as the AMP plugin. */
,

	/* Plugin Name of the plugin */
	__( 'AMP', 'amp' ),

This syntax error doesn't cause a problem for the purposes of translating on WordPress.org since the file isn't actually executed, but rather the gettext function calls are just extracted.

However, the presence of this syntax error causes problems when attempting to deploy a build of the plugin to a hosting environment where PHP syntax checking is part of a pre-receive hook, so a git push can then fail. So far this is known to be an issue on WP Engine and it would also be a problem on WordPress.com VIP.

@westonruter westonruter added this to the v1.0 milestone Sep 10, 2018
@westonruter
Copy link
Member Author

@swissspidy I recall you mentioning that there was a syntax error previously and that it was a known issue. Is there any workaround you are aware of?

@swissspidy
Copy link
Collaborator

You're right. I noted this syntax error in #1329 (comment) but didn't realize it could cause issues with deployments.

We might be able to work around this by using something like wp i18n make-pot --file-comment="". I'll try this out.

Otherwise we could a) strip the comma after the file generation or b) fix the underlying problem in https://github.com/WordPress/gutenberg/blob/410f78a705a8aca2ca32a225383dc48cf47e0fe4/packages/i18n/tools/pot-to-php.js

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 a pull request may close this issue.

2 participants