Skip to content
This repository has been archived by the owner on Aug 13, 2023. It is now read-only.

Add script boilerplate for amp js #2433

Merged
merged 22 commits into from
Oct 23, 2019
Merged

Add script boilerplate for amp js #2433

merged 22 commits into from
Oct 23, 2019

Conversation

samora
Copy link
Contributor

@samora samora commented Oct 18, 2019

Partially resolves bbc/simorgh#2156

Overall change: Add amp JS scripts boilerplate to psammead-assets. See bbc/simorgh#2156


  • I have assigned myself to this PR and the corresponding issues
  • Automated jest tests added (for new features) or updated (for existing features)

Copy link
Contributor

@dr3 dr3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good :) Just needs some snapshot tests added :)

Copy link
Contributor

@hotinglok hotinglok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jenkins is yelling about stuff, but overall LGTM.

@amywalkerdev
Copy link
Contributor

Tests are failing in packages/utilities/psammead-assets/index.test.jsx

Psammead assets › should test all the utility exports exist and are the correct type for main

Missing value 'AMP_JS' in the expected export for 'psammead-assets/amp-boilerplate'.

Psammead assets › should test all the utility exports exist and are the correct type for module

Missing value 'AMP_JS' in the expected export for 'psammead-assets/amp-boilerplate'.

@amywalkerdev
Copy link
Contributor

Change scanner is complaining:

Branch must update CHANGELOG.md in psammead
Branch must update package.json in psammead

I would recommend removing your changes to the base package-lock.json as they are not changing anything useful :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve script tag source for amp analytics
9 participants