Skip to content

Conversation

fraserxu
Copy link
Contributor

This pull request trying to fix two thing:

  1. Remove default cache-control header Remove default cache-control header #1
  2. Only set cache-control headers when the response statusCode is 2xx as we don't want to cache error response in most case.

.end(done);
});

it('sets cache control to 5 minutes? by default', function(done) {
Copy link
Member

Choose a reason for hiding this comment

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

Why did you delete these tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, so if we don't set default cache-control header, what we should check here in the test? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

There's a default set to 5 minutes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@scottcorgan It's intended to remove the default cache-headers for the same reason as #1 . The current code will override whatever is set inline which might not work as expected.

@scottcorgan scottcorgan merged commit 72fb88a into divshot:master Dec 6, 2016
@scottcorgan
Copy link
Member

Thanks. Published as 2.0.0.

@fraserxu
Copy link
Contributor Author

fraserxu commented Dec 6, 2016

Thank you! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants