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

Update allowed tags according to AMP specification #804

Closed
westonruter opened this issue Nov 13, 2017 · 1 comment

Comments

Projects
3 participants
@westonruter
Copy link
Member

commented Nov 13, 2017

[Edited] Refer to the latest acceptance criteria.


It appears that the amp_wp_build.py script hasn't been run since it was introduced at the end of 2016. Since that time, the changes to class-amp-allowed-tags-generated.php appear to have been done manually. There have been 95 commits to the spec since the end of 2016, and these changes need to be comprehensively added to the plugin.

I understand the amp_wp_build.py script has to be updated to account for some changes to the spec and to the amphtml project. For example, a dependency has been removed which has to be reverted or restored in some way: ampproject/amphtml#10830

Additionally, the spec changes need to be more regularly applied, such as via Travis CI cron, if not done manually.

@westonruter westonruter added this to the v0.6 milestone Nov 13, 2017

@westonruter westonruter added this to To Do in v0.6 via automation Nov 13, 2017

@ThierryA

This comment has been minimized.

Copy link
Collaborator

commented Nov 23, 2017

Acceptance criteria:

  1. Conduct a Discovery to fully determine how the existing script is working.
  2. Based on the findings of the Discovery, determine the best way to fix the components/functions of the script that are either broken or not working as intended.
  3. More details to come post Discovery...

Additional acceptance criteria:
4. Create the new allowed tags file, class-amp-allowed-tags-generated.php
5. Open a pull request to the plugin repo with that new file, updated PHPUnit tests, and this commit to prevent an error in accessing an undefined property and update the extensions/paths 6. Submit a pull request to amphtml to revert the deletion of validator_gen_md.py, making this commit.
6. Create a bin/update-allowed-tags script that runs everything, including composer etc.

@kienstra kienstra self-assigned this Nov 28, 2017

@ThierryA ThierryA moved this from To Do to In Progress in v0.6 Nov 28, 2017

@kienstra kienstra moved this from In Progress to Ready for Review in v0.6 Dec 9, 2017

@westonruter westonruter moved this from Ready for Review to QA in v0.6 Dec 12, 2017

@csossi csossi moved this from QA to Ready for Merging in v0.6 Dec 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.