Skip to content

🚀 [Story performance] Launch first page load experiment to 10%#35192

Merged
mszylkowski merged 3 commits into
ampproject:mainfrom
mszylkowski:launch_firstpage
Jul 13, 2021
Merged

🚀 [Story performance] Launch first page load experiment to 10%#35192
mszylkowski merged 3 commits into
ampproject:mainfrom
mszylkowski:launch_firstpage

Conversation

@mszylkowski
Copy link
Copy Markdown
Contributor

Launches the story-load-first-page-only experiment to 10%.
Also, since experiments don't start with amp- (everything is amp), removed the prefix amp- from the experiment name

@mszylkowski mszylkowski requested a review from gmajoulet July 12, 2021 19:25
@amp-owners-bot amp-owners-bot Bot requested a review from jpettitt July 12, 2021 19:25
@amp-owners-bot
Copy link
Copy Markdown

Hey @gmajoulet, @newmuis! These files were changed:

extensions/amp-story/1.0/amp-story.js
extensions/amp-story/1.0/test/test-amp-story.js

Copy link
Copy Markdown
Contributor

@kristoferbaxter kristoferbaxter left a comment

Choose a reason for hiding this comment

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

Left a nit for a future consideration.

if (isExperimentOn(this.win, 'story-load-first-page-only')) {
Services.performanceFor(this.win).addEnabledExperiment(
'amp-story-load-first-page-only'
'story-load-first-page-only'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: This appears a bit strange, why does amp-story read for the value of an experiment followed by only setting the same value for elsewhere?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please let us know if we're missing something:
The CSI pipeline doesn't track experiments by defaults, we need to specifically enable them through this method so they appear in our dashboards. We'll want to monitor the LCP improvements for this experiment before launching fully.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't think you're missing anything, but we should revisit this later on.

CSI should automatically insert all tests so we don't have to remember to do this.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants