Skip to content

🔥 Revert "📈 Initial StorySpec Implementation (#23030)"#24013

Merged
Enriqe merged 3 commits intoampproject:masterfrom
Enriqe:revert-storyspec
Aug 16, 2019
Merged

🔥 Revert "📈 Initial StorySpec Implementation (#23030)"#24013
Enriqe merged 3 commits intoampproject:masterfrom
Enriqe:revert-storyspec

Conversation

@Enriqe
Copy link
Copy Markdown
Contributor

@Enriqe Enriqe commented Aug 16, 2019

Fixes #24017

We just noticed some early analytic events (e.g. story-page-visible on the first page) aren't being logged correctly since the amp-analytics service isn't ready to listen for these events yet. This has been happening since #23030 was pushed since it stopped buffering early events.

This reverts commit 1a3eb26. #23030

@zhouyx
Copy link
Copy Markdown
Contributor

zhouyx commented Aug 16, 2019

@Enriqe I don't quite understand why we are reverting the feature instead of fixing it.

By reverting the feature, it's will be a breaking change to whoever is using this config. Is that expected? Thanks

@Enriqe
Copy link
Copy Markdown
Contributor Author

Enriqe commented Aug 16, 2019

@zhouyx we decided it was safer to rollback than to fix it. We haven't documented the config yet so we don't expect anyone to be relying on this yet.

/cc @newmuis

@zhouyx
Copy link
Copy Markdown
Contributor

zhouyx commented Aug 16, 2019

Talked to @Enriqe offline. It's safer to rollback if we are going to patch to prod. But please test manually given the original PR was merged a long time ago.
LGTM

@Enriqe Enriqe merged commit 2918462 into ampproject:master Aug 16, 2019
dreamofabear pushed a commit that referenced this pull request Aug 16, 2019
* Revert "📈 Initial StorySpec Implementation (#23030)"

This reverts commit 1a3eb26.

* fix import

* fix test
dreamofabear pushed a commit that referenced this pull request Aug 16, 2019
* Revert "📈 Initial StorySpec Implementation (#23030)"

This reverts commit 1a3eb26.

* fix import

* fix test
westonruter added a commit to westonruter/amphtml that referenced this pull request Aug 17, 2019
…cript-img-with-http-protocol

* 'master' of github.com:ampproject/amphtml: (1326 commits)
  Fix and enable e2e tests for AMPHTML ads FIE rendering mode (ampproject#23995)
  🏗 Update WorkerDOM to 0.17.0 (ampproject#24024)
  Make DocInfo.pageViewId64 async (ampproject#23998)
  🐛 Updates amp-sidebar in amp-story  (ampproject#23956)
  Revert "Revert "📖Update documentation for carousel 0.2 (ampproject#23840)" (ampproject#23967)" (ampproject#24016)
  🔥 Revert "📈 Initial StorySpec Implementation (ampproject#23030)" (ampproject#24013)
  Extension skeleton code for payment widgets (ampproject#23045)
  🏗🐛 Don't call `travisBuildNumber()` in the global scope (ampproject#24021)
  Remove suppressTypes from amp-mustache. (ampproject#23993)
  🐛 Move `terser` from `dependencies` to `devDependencies` (ampproject#24018)
  Revert "Revert "Set the new loaders experiment to 1% of traffic. (ampproject#23780)" (ampproject#23963)" (ampproject#24014)
  SwG release 0.1.22.63 (ampproject#23997)
  Resolve navTiming variable earlier if possible (ampproject#23580)
  🏗 Don't run all the runtime tests for validator-only changes (ampproject#24010)
  Collect document ready signal (ampproject#23981)
  Validator rollup (ampproject#24000)
  Remove flaky story branching test. (ampproject#23994)
  Include amp-base-carousel in amp-carousel's build. (ampproject#23984)
  Partial validator rollup (ampproject#23996)
  amp-bind: Rate-limit history operations (ampproject#23938)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Some analytics for stories are broken

4 participants