Skip to content

♻️ Extract json helpers to core/types/object/json#34367

Merged
rcebulko merged 15 commits intoampproject:mainfrom
rcebulko:core-json
May 14, 2021
Merged

♻️ Extract json helpers to core/types/object/json#34367
rcebulko merged 15 commits intoampproject:mainfrom
rcebulko:core-json

Conversation

@rcebulko
Copy link
Copy Markdown
Contributor

@rcebulko rcebulko commented May 13, 2021

This is the diff to review: 2179341...54ada92
Everything else is just updating import statements.

Part of #32693

Also they pass type-checking now

@amp-owners-bot
Copy link
Copy Markdown

amp-owners-bot bot commented May 13, 2021

Hey @erwinmombay! These files were changed:

build-system/eslint-rules/json-configuration.js
build-system/eslint-rules/no-has-own-property-method.js

Hey @jridgewell! These files were changed:

build-system/eslint-rules/json-configuration.js
build-system/eslint-rules/no-has-own-property-method.js
src/core/data-structures/finite-state-machine.js
src/core/data-structures/promise.js
src/core/data-structures/signals.js
src/core/error.js
src/core/types/object/index.js
src/core/types/object/json.extern.js
src/core/types/object/json.js

Hey @Jiaming-X! These files were changed:

extensions/amp-ad-network-adsense-impl/0.1/responsive-state.js

Hey @jeffkaufman! These files were changed:

extensions/amp-ad-network-adsense-impl/0.1/responsive-state.js
extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js
extensions/amp-ad-network-doubleclick-impl/0.1/safeframe-host.js

Hey @alanorozco! These files were changed:

extensions/amp-connatix-player/0.1/amp-connatix-player.js

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

extensions/amp-story-panning-media/0.1/amp-story-panning-media.js
extensions/amp-story/1.0/amp-story-access.js
extensions/amp-story/1.0/amp-story-consent.js
extensions/amp-story/1.0/amp-story-store-service.js
extensions/amp-story/1.0/history.js
extensions/amp-story/1.0/jsonld.js
src/localized-strings.js

Hey @processprocess! These files were changed:

extensions/amp-story-panning-media/0.1/amp-story-panning-media.js

Hey @Enriqe! These files were changed:

extensions/amp-story/1.0/amp-story-access.js
extensions/amp-story/1.0/amp-story-consent.js
extensions/amp-story/1.0/amp-story-store-service.js
extensions/amp-story/1.0/history.js
extensions/amp-story/1.0/jsonld.js
src/amp-story-player/amp-story-player-impl.js
src/localized-strings.js

@@ -0,0 +1,351 @@
/**
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was moved from test/unit/core/types/test-object.js

@@ -1,5 +1,5 @@
/**
* Copyright 2015 The AMP HTML Authors. All Rights Reserved.
* Copyright 2021 The AMP HTML Authors. All Rights Reserved.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

GH doesn't show this, but this file was moved to src/core/object/types/json.js (which has the 2015 copyright date) and this file is "new"

Copy link
Copy Markdown
Member

@danielrozenberg danielrozenberg left a comment

Choose a reason for hiding this comment

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

Approval for build-system/*

@rcebulko rcebulko enabled auto-merge (squash) May 13, 2021 18:20
@rcebulko rcebulko disabled auto-merge May 13, 2021 20:07
This was referenced Nov 5, 2021
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.

6 participants