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

Use accept header that is sorted for default header. #429

Merged
merged 3 commits into from
Nov 15, 2020

Conversation

jksy
Copy link
Contributor

@jksy jksy commented Oct 28, 2020

Goals ⚽

I'm building an image storage SaaS and using a CDN.
One of my clients is using this library and the AcceptHeader changes every time.
So the CDN cache doesn't work effectively.

Implementation Details 🚧

I change the order of the Accept Header to sort before return it.

Testing Details 🔍

I've modified it to test the sorted accept header.
Please let me know what kind of test I should do if needed other cases.

@jksy jksy marked this pull request as draft October 31, 2020 05:25
@jksy
Copy link
Contributor Author

jksy commented Oct 31, 2020

@jshier @cnoon
Hi,
One test is failing, but I think it has something to do with the matter below.
#415

I'd like to merge to master with this PR, but how do I do it?

@jksy jksy marked this pull request as ready for review October 31, 2020 12:29
Copy link
Contributor

@jshier jshier left a comment

Choose a reason for hiding this comment

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

Thanks!

@jshier jshier merged commit 790201e into Alamofire:master Nov 15, 2020
@jksy jksy deleted the feature/sorted-accept-header branch November 16, 2020 02:00
@jshier jshier added this to the 4.2.0 milestone Apr 4, 2021
ryndinol pushed a commit to ryndinol/AlamofireImage that referenced this pull request Apr 26, 2021
* use accept header that is sorted

* fix tests

* fix UIButtonTest for this changes

Co-authored-by: Junichiro Kasuya <[email protected]>
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