Update the version number for amp-story to 1.0 #14420
Conversation
|
@newmuis You mention aliasing 0.1 as 1.0 but then mention copying 1.0 back to 0.1. Will it not remain an alias at that point (e.g. both 0.1 and 1.0 serve the same code)? |
| beforeEach(() => { | ||
| win = env.win; | ||
| element = win.document.createElement('amp-story'); | ||
| // element.getAmpDoc = () => env.ampdoc; |
There was a problem hiding this comment.
@choumx @erwinmombay mocking this all seems to work, but seems odd to have to do
|
@honeybadgerdontcare we will remove the alias and continue development on 1.0 and not 0.1, so they will diverge over time. |
|
The tests are failing because they keep loading the As per https://github.com/ampproject/amphtml/blob/master/testing/describes.js#L692, unless you provide a specific extension version, it always use To fix the tests, we need to change with |
|
Thanks @gmajoulet for tracking this down! |
| @@ -0,0 +1,453 @@ | |||
| /** | |||
There was a problem hiding this comment.
It looks like this file (+ its CSS and test files) got deleted, should we remove them from this migration?
There was a problem hiding this comment.
Looks like we also have references to the 0.1 version is this new https://github.com/ampproject/amphtml/blob/master/extensions/amp-story-auto-ads/0.1/amp-story-auto-ads.js file
|
@honeybadgerdontcare can you verify the validator changes? |
|
@newmuis github is saying theres conflicts for the test validator files, can you fix? the protoascii change looks fine. if/when 1.0 starts to drift from 0.1 we may need to be thoughtful on how to split up the extension script for 0.1 and 1.0 as I imagine 1.0 usage attributes may change and we'll want that tag usage to rely on 1.0 extension and not 0.1 extension. |
|
Resolved the validation merge conflicts. Any other feedback @honeybadgerdontcare or @choumx? |
| <script async custom-element="amp-video" src="https://cdn.ampproject.org/v0/amp-video-0.1.js"></script> | ||
| <script async custom-element="amp-audio" src="https://cdn.ampproject.org/v0/amp-audio-0.1.js"></script> | ||
| <script async custom-element="amp-story" src="https://cdn.ampproject.org/v0/amp-story-0.1.js"></script> | ||
| <script async custom-element="amp-analytics" src="https://cdn.ampproject.org/v0/amp-analytics-0.1.js"></script> |
There was a problem hiding this comment.
is it intentional to remove amp-analytics here too?
There was a problem hiding this comment.
Yes, amp-analytics is unused in this example.
…he amp-story extension.
* Fork to 1.0 * Migrate request service and change examples * Update AMP.registerElement calls to be in AMP.extension callback. * Add back 0.1 version to validator * Update test-amp-story to use 1.0 * Update origin whitelist and amp-story-cta-layer tests to use 1.0 of the amp-story extension. * Update amp-story-auto-ads's amp-story deps to 1.0 * Update missing 0.1 => 1.0 renames * Change forbidden dep whitelist to use amp-story 1.0. * Move added files * Update test-full-bleed-animations to use 1.0 * Update validator out file error file paths to 1.0
This is a first step towards fixing #14357. We move amp-story from 0.1 => 1.0, aliasing 0.1 as 1.0. As a separate PR, we will copy 1.0 back to 0.1 to freeze it; this extra step is necessary to retain history properly.