Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Plek#asset_root over Plek#public_asset_host #1802

Merged
merged 1 commit into from
Jun 22, 2020

Conversation

kevindew
Copy link
Member

Trello: https://trello.com/c/oNEtjIVp/99-serve-static-assets-from-www-hostname

I'm removing this as part of a plan to remove Plek#public_asset_host and
the associated GOVUK_ASSET_HOST env var. The reason to remove these is
because they are the same #asset_root and GOVUK_ASSET_ROOT, which pushes
developers to consider which one to use when they both have the same
effect.

Government frontend appears to be the only app other than Whitehall that
has used this method, which I assume is because the code it is used in
was ported from Whitehall.


Visual regression results:
https://government-frontend-pr-[THIS PR NUMBER].surge.sh/gallery.html

I'm removing this as part of a plan to remove Plek#public_asset_host and
the associated GOVUK_ASSET_HOST env var. The reason to remove these is
because they are the same #asset_root and GOVUK_ASSET_ROOT, which pushes
developers to consider which one to use when they both have the same
effect.

Government frontend appears to be the only app other than Whitehall that
has used this method, which I assume is because the code it is used in
was ported from Whitehall.
@kevindew kevindew requested a review from thomasleese June 22, 2020 14:15
@bevanloon bevanloon temporarily deployed to government-f-remove-pub-rxdli1 June 22, 2020 14:15 Inactive
@kevindew kevindew merged commit c8f6522 into master Jun 22, 2020
@kevindew kevindew deleted the remove-public-asset-host branch June 22, 2020 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants