Skip to content

build-system: Support custom config overlay#27977

Merged
mdmower merged 4 commits intoampproject:masterfrom
mdmower:pr-config-overlay
Apr 25, 2020
Merged

build-system: Support custom config overlay#27977
mdmower merged 4 commits intoampproject:masterfrom
mdmower:pr-config-overlay

Conversation

@mdmower
Copy link
Copy Markdown
Contributor

@mdmower mdmower commented Apr 23, 2020

Provide a mechanism for overlaying AMP_CONFIG at build time.

If build-system/global-configs/custom-config.json exists and is valid JSON, overlay the active config JSON before applying it to the target script. This feature would allow anyone building AMP to tweak the applied config without modifying checked-in source. For example, build-system/global-configs/custom-config.json could contain

{
    "cdnUrl": "https://example.com/amp",
    "geoApiUrl": "https://example.com/geo"
}

and these properties would be added to AMP_CONFIG at build time.

This feature would be documented in the hosting guide (PR #27100).

Provide a mechanism for overlaying the AMP_CONFIG applied to built
scripts.

If `build-system/global-configs/custom-config.json` exists and
is valid JSON, overlay the active config JSON before applying.
@amp-owners-bot amp-owners-bot bot requested a review from rcebulko April 23, 2020 05:44
@mdmower mdmower requested review from jridgewell and rsimha April 23, 2020 05:44
@mdmower
Copy link
Copy Markdown
Contributor Author

mdmower commented Apr 23, 2020

This PR is inspired by @jridgewell's comment in PR #27100:

Shouldn't we be requesting they modify the AMP_CONFIG injected when creating an RTV? That way you don't need to maintain the modified config.js with possibly conflicting upstream changes.

Copy link
Copy Markdown
Contributor

@rcebulko rcebulko left a comment

Choose a reason for hiding this comment

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

LGTM but leaving un-approved to ensure one of @rsimha or @jridgewell get their eyes on it

@mdmower
Copy link
Copy Markdown
Contributor Author

mdmower commented Apr 24, 2020

Thanks for the reviews!

@mdmower mdmower merged commit df7c93a into ampproject:master Apr 25, 2020
@mdmower mdmower deleted the pr-config-overlay branch April 25, 2020 00:11
ldoroshe pushed a commit to ldoroshe/amphtml that referenced this pull request May 8, 2020
* build-system: Support custom config overlay

Provide a mechanism for overlaying AMP_CONFIG at build time.

If build-system/global-configs/custom-config.json exists and is valid JSON, overlay the active config JSON before applying it to the target script. This feature would allow anyone building AMP to tweak the applied config without modifying checked-in source. For example, build-system/global-configs/custom-config.json could contain

{
    "cdnUrl": "https://example.com/amp",
    "geoApiUrl": "https://example.com/geo"
}
and these properties would be added to AMP_CONFIG at build time.

* Update readme

* Revisions from review
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