-
Notifications
You must be signed in to change notification settings - Fork 385
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
Add readme.txt transformation from README.md #5815
Conversation
Plugin builds for 6222a32 are ready 🛎️!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ship it.
-1, | ||
$replace_count | ||
); | ||
if ( 0 === $replace_count ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition will never be true as $replace_count
will always have a value > 0 after the screenshot transformations above, and preg_replace_callback()
will increase that previous value by the number of replacements done here. So, the condition should either be $replace_count === $replace_count
, or use an unset variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem to be the case: https://3v4l.org/0CGoT
<?php
$str = 'abcba';
$str = preg_replace( '/b/', 'a', $str, -1, $replace_count );
var_dump( $replace_count );
$str = preg_replace( '/c/', 'a', $str, -1, $replace_count );
var_dump( $replace_count );
var_dump( $str );
The result is:
int(2)
int(1)
string(5) "aaaaa"
In other words, $replace_count
is reset with each preg_replace_callback()
. It doesn't increment on top of an existing variable. If it did, then the int(3)
would be here instead of int(1)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea I've been terribly mistaken here. I was operating against changes I made locally 🤦. This is all good.
Co-authored-by: Pierre Gordon <[email protected]>
QA Passed No 11c11
< The Official AMP Plugin, supported by the AMP team. Formerly Accelerated Mobile Pages, AMP enables great experiences across both mobile and desktop.
---
> The official AMP Plugin, supported by the AMP team. Formerly Accelerated Mobile Pages, AMP enables great experiences across both mobile and desktop.
15c15,21
< This official plugin from the AMP project enables AMP content publishing with WordPress in a way that is fully and seamlessly integrated with the standard mechanisms of the platform. The key features are the following:
---
> This official plugin from the AMP project enables AMP content publishing with WordPress in a way that is fully and seamlessly integrated with the standard mechanisms of the platform.
>
> https://www.youtube.com/watch?v=s52JNMT59s8&list=PLXTOW_XMsIDRGRr5QDffrvND8Qh1RndFb
>
> For more videos like this, check out the ongoing [AMP for WordPress video series](https://www.youtube.com/playlist?list=PLXTOW_XMsIDRGRr5QDffrvND8Qh1RndFb).
>
> The plugin's key features include:
60c66
< If you are a developer, we encourage you to [follow along](https://github.com/ampproject/amp-wp) or [contribute](https://github.com/ampproject/amp-wp/blob/develop/contributing.md) to the development of this plugin on GitHub.
---
> If you are a developer, we encourage you to [follow along](https://github.com/ampproject/amp-wp) or [contribute](https://github.com/ampproject/amp-wp/wiki/Contributing) to the development of this plugin on GitHub. |
This is a follow-up on #5807 for #5791.
Relying on the
README.md
alone had some downsides:AMP Plugin for WordPress
) and had to resort to the ambiguousAMP
.<br>
between the caption and the image link.This PR addresses these problems by introducing an automated transformer from
README.md
toreadme.txt
as part of the build step. In contrast to before, thereadme.txt
is not committed to version control, and as such it is no longer needing to be maintained separately.Here's the difference between the
readme.txt
currently on WordPress.org with the newreadme.txt
that this PR's transformer generates:In short, the
README.md
is now canonical, and thereadme.txt
is generated from it at build time.The
README.md
is basically restored to what we were previously generating fromreadme.txt
: