Added cache control to readme files #1924
Conversation
`extra_headers` no longer wrapped in Option<T>
|
r? @smarnach (rust_highfive has picked a reviewer for you, use r? to override) |
src/bin/render-readmes.rs
Outdated
| @@ -137,7 +137,7 @@ fn main() { | |||
| content, | |||
| content_length, | |||
| "text/html", | |||
| None, | |||
| header::HeaderMap::new(), | |||
There was a problem hiding this comment.
Shouldn't we set a TTL here as well?
(For context, this is a binary that re-renders all readme files. It can be manually run if we make updates to the readme rendering code.)
| let mut extra_headers = header::HeaderMap::new(); | ||
| extra_headers.insert( | ||
| header::CACHE_CONTROL, | ||
| CACHE_CONTROL_IMMUTABLE.parse().unwrap(), |
There was a problem hiding this comment.
The variable CACHE_CONTROL_IMMUTABLE is set to public,max-age=31536000,immutable, i.e. the files are tagged to never change and to live for one year in the cache. However, the readme files might change, so let's cache them only for a week, and let's not tag them as immutable. (You may want to read the documentation of the cache-control header).
There was a problem hiding this comment.
I see, sorry for misunderstanding the original Issue. So I assume the only field that is required is max-age=<seconds>?
There was a problem hiding this comment.
I think you can keep public as well. Not sure whether it has any effect when setting max-age at the same time, but it certainly doesn't hurt.
Now no longer lasts for a year and removes immutability
|
@bors r+ This should be low risk to deploy. |
|
📌 Commit f82d6e3 has been approved by |
Added cache control to readme files Related issue: #1915 `extra_headers` parameter is no longer wrapped in `Option<T>`
|
☀️ Test successful - checks-travis |
Related issue: #1915
extra_headersparameter is no longer wrapped inOption<T>