-
Notifications
You must be signed in to change notification settings - Fork 61
Add optional headers to purge call #118
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
Conversation
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.
we should also say that you should only use this when the header varies. if its always the same (e.g. a basic authentication or similar), you should just pass in a guzzle client configured to send that header, rather than spread the logic throughout the code.
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.
Added a section to the docs.
|
i think its consistent to do this as we have it for refresh. its a special case - the more common case of always having to send some headers (to authenticate or such) should be handled by passing a custom guzzle client. |
|
Added doc section. As this doesn’t break BC – optional interface method arguments can be left out of implementations (nice one, PHP) – this can be safely merged. |
|
are you sure? i thought php complains about that. but well, we are still RC so its like last moment ;-) |
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.
does the link really work like this? don't you need the filename in <>?
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.
@dbu it is a ref to the line 182 in this file.
Add optional headers to purge call
|
thanks a lot! |
Fix #117.
I think this makes sense, but @dbu, please validate whether this answers to a proper use case (#117).