-
-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add Cache-Control #9
Conversation
whoa, the test fail? |
index.js
Outdated
@@ -74,5 +75,6 @@ fetchRepos(); | |||
|
|||
module.exports = (request, response) => { | |||
controlAccess()(request, response); | |||
response.setHeader("cache-control", cache) |
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.
Single quotes
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.
done
index.js
Outdated
@@ -5,6 +5,7 @@ const controlAccess = require('control-access'); | |||
const token = process.env.GITHUB_TOKEN; | |||
const username = process.env.GITHUB_USERNAME; | |||
const origin = process.env.ACCESS_ALLOW_ORIGIN; | |||
const cache = "max-age=" + (process.env.CACHE_MAX_AGE || 300); |
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.
Template string
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.
done (on etag commit)
single quotes and semicolons
You could add ETag too, which has much more effect, especially since the repos are only updates once a day. |
Looks good. Thank you :) |
Add HTTP Header
cache-control
so that browsers can cache requests for short time, thus saving bandwidth in the long term.Also adds CACHE_MAX_AGE optional environment variable in case someone wish to modify that.