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

Add Cache-Control header #336

Merged
merged 2 commits into from Sep 4, 2020
Merged

Add Cache-Control header #336

merged 2 commits into from Sep 4, 2020

Conversation

Antolius
Copy link
Contributor

Add Cache-Control response header for static files in order to improve page loading speed.
Fixes #322


  • I have signed the CLA

@Antolius
Copy link
Contributor Author

Hi! I'm new to the code-base, so might have missed something obvious, but this seems rather straightforward.

I've also noticed a TODO comment on the InitStaticRoutes method that says it should be converted into a func. If you'd like I can just update the method into something like this:

func InitStaticRoutes(app *App, r *mux.Router)

and leave its implementation as-is.

Or, if the comment had some broader implications I could try working on them as well. For example, it seems like shttp field in the App struct could be removed, in which case the InitStaticRoutes function would not need to modify the app passed into it. This might be a bit bigger change, so it might be better to have it in a separate pull request.

Copy link
Member

@thebaer thebaer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking at this! And sorry for not responding earlier.

Overall, everything looks great! I have just a small suggestion to help keep things organized.

Otherwise, I really appreciate you including a test with these changes. As for that TODO and getting rid of the shttp field, please feel free to submit a separate PR with those changes -- we'd very much welcome those improvements!

cache.go Outdated Show resolved Hide resolved
@thebaer thebaer added this to the 0.13 milestone Sep 1, 2020
Add Cache-Control response header for static files in order to improve page loading speed.
Fixes  #322
Copy link
Member

@thebaer thebaer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks!

@thebaer thebaer merged commit c31a87f into writefreely:develop Sep 4, 2020
@Antolius Antolius deleted the static-assets-cache-control branch September 4, 2020 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

static assets served without cache policy leads to suboptimal page speed
2 participants