Skip to content

Only throw deprecation when user explicitly sets option #38

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

Merged
merged 1 commit into from
Apr 5, 2017

Conversation

acrobat
Copy link
Contributor

@acrobat acrobat commented Apr 5, 2017

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets fixes #37
Documentation /
License MIT

What's in this PR?

This pr fixes the issue so the deprecation notice is only throw when the users sets the "respect_cache_headers" option.

Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

Thank you

@@ -338,7 +338,7 @@ private function configureOptions(OptionsResolver $resolver)
'cache_lifetime' => 86400 * 30, // 30 days
'default_ttl' => 0,
//Deprecated as of v1.3, to be removed in v2.0. Use respect_response_cache_directives instead
'respect_cache_headers' => true,
'respect_cache_headers' => null,
Copy link
Member

Choose a reason for hiding this comment

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

You do also have to add "null" as an allowed type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes indeed, fixed!

@acrobat acrobat force-pushed the deprecation-notice-fix branch from e7f36e6 to a1c0b7d Compare April 5, 2017 18:59
@Nyholm Nyholm merged commit 3b72fd7 into php-http:master Apr 5, 2017
@Nyholm
Copy link
Member

Nyholm commented Apr 5, 2017

Thank you

@acrobat acrobat deleted the deprecation-notice-fix branch April 5, 2017 19:13
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.

A deprecation notice is always triggered
2 participants