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

Comments

Projects
None yet
2 participants
@westonruter
Member

westonruter commented Sep 10, 2018

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

This comment has been minimized.

Member

westonruter commented Sep 10, 2018

@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

This comment has been minimized.

Collaborator

swissspidy commented Sep 10, 2018

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

swissspidy added a commit that referenced this issue Sep 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment