Skip to content
This repository has been archived by the owner on Jan 28, 2022. It is now read-only.

allow resource metadata to be explicitly set in config file #26

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

viz
Copy link

@viz viz commented Sep 7, 2016

We needed to set max-age=0 to prevent Cloudfront from caching in dev/stage.

This simple change allows the default headers to be set in the config file.

Sorry, haven't had time to create any tests.

@LeZuse
Copy link
Member

LeZuse commented Sep 7, 2016

This is great! Thanks for contributing back!

Just curious, is there a reason you don't include hashes in your file names? That effectively solves your usecase and also makes the deploy faster to upload.

Sorry about the failing checks here 😕, I really need to make some more time to clean this project up a bit.

@viz
Copy link
Author

viz commented Sep 7, 2016

Hey Tomas,

For this particular use, we're deploying a React component to S3 and using it in a Wordpress page. So we actually want the filename to be the same because we don't want to have to update the script tag in Wordpress every time we release an update to the component.

In other cases webpack is including the hash in the filename since we're generating the script tag when we know the fingerprinted filename.

Cheers,

Pete

Sent from my iPad

On 7 Sep 2016, at 10:58 AM, Tomas Ruzicka [email protected] wrote:

This is great! Thanks for contributing back!

Just curious, is there a reason you don't include hashes in your file names? That effectively solves your usecase and also makes the deploy faster to upload.

Sorry about the failing checks here 😕, I really need to make some more time to clean this project up a bit.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@LeZuse
Copy link
Member

LeZuse commented Sep 7, 2016

Makes total sense. Will look on merging this sometime next week. We have some urgent stuff to do in our team now.

@LeZuse LeZuse self-assigned this Sep 7, 2016
@viz
Copy link
Author

viz commented Sep 7, 2016

No problem. We're using our fork for now so just let me k ow when you've updated npm and we'll switch back.

Sent from my iPhone

On 7 Sep 2016, at 10:41 PM, Tomas Ruzicka [email protected] wrote:

Makes total sense. Will look on merging this sometime next week. We have some urgent stuff to do in our team now.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants