-
Notifications
You must be signed in to change notification settings - Fork 192
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
Adjusting import map #1722
Adjusting import map #1722
Conversation
@tuxpiper Ready for review! |
@tuxpiper I somehow messed up something when building it looks like, I'll let you know when I have figured it out... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Note to self: need to work on the routine that copies the files to the CDN, so that it assigns /importmap.json non-caching HTTP headers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking even better now :) thinking to apply these headers to the different files:
- include: [ index.html, config.js, config.json, manifest.json, importmap.json ]
cache_control: "public, max-age=0, s-maxage=0, must-revalidate"
- include: [ locales/* ]
cache_control: "public, max-age=60, s-maxage=60, stale-while-revalidate=86400"
- exclude: [ index.html, config.js, config.json, manifest.json, importmap.json, locales/* ]
cache_control: "public, max-age=31536000, s-maxage=31536000"
I've added special rules for the locales files, which will allow them to be used for one full day, while still checking for changes in the background.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, still lgtm 👍
@Obadha2 Ready for QA. Another smoke test is needed, and could you also include some tests with the change of language? I did some changes there. 🙏 |
This pull request makes the following changes:
Testing checklist:
[ ]
I certify that I ran my checklist
Fixes ushahidi/platform# .
Ping @ushahidi/platform