Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Clarify documentation for supported encodings in AFJSONResponseSerializer #3750

Merged
merged 3 commits into from
Nov 24, 2017
Merged

Conversation

skyline75489
Copy link
Contributor

@skyline75489 skyline75489 commented Oct 13, 2016

This clarify the data encoding needed for AFJSONResponseSerializer to successfully parsing the response as JSON object.

@kcharwood kcharwood changed the title Clarify documentation for supported encodings in AFJSONResponseSerial… Clarify documentation for supported encodings in AFJSONResponseSerializer Oct 13, 2016
@kcharwood kcharwood added this to the 3.2.0 milestone Oct 13, 2016
@@ -111,6 +111,8 @@ NS_ASSUME_NONNULL_BEGIN
- `application/json`
- `text/json`
- `text/javascript`

According to RFC 4627, JSON text should be encoded in Unicode and the default encoding is UTF-8. NSJSONSerialization also recommends using UTF-8 for efficiency, even though it actually supports more encodings. Using unsupported encoding will result in serialization error. See the `NSJSONSerialization` documentation for more details.
Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to mention RFC 7159 instead of RFC 4627 which is an older version. Also, it’s not should, it’s SHALL which, according to RFC 2119 is is an absolute requirement of the specification.

@@ -111,6 +111,8 @@ NS_ASSUME_NONNULL_BEGIN
- `application/json`
- `text/json`
- `text/javascript`

In RFC 7159 - Section 8.1, it states that JSON text is required to be encoded in UTF-8, UTF-16, or UTF-32, and the default encoding is UTF-8. NSJSONSerialization provides support for all the encodings listed in the specification, and recommends UTF-8 for efficiency. Using unsupported encoding will result in serialization error. See the `NSJSONSerialization` documentation for more details.
Copy link
Contributor

Choose a reason for hiding this comment

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

Small typo: should say “using an unsupported encoding.”

@SlaunchaMan
Copy link
Contributor

@skyline75489 Can you merge in master or rebase off of it to get the build to pass on Travis?

@codecov-io
Copy link

codecov-io commented Nov 23, 2017

Codecov Report

Merging #3750 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #3750   +/-   ##
======================================
  Coverage    82.8%   82.8%           
======================================
  Files          46      46           
  Lines        5304    5304           
  Branches      439     439           
======================================
  Hits         4392    4392           
  Misses        671     671           
  Partials      241     241
Impacted Files Coverage Δ
AFNetworking/AFURLResponseSerialization.h 90% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d5a894...8ad7050. Read the comment docs.

@skyline75489
Copy link
Contributor Author

@SlaunchaMan Done.

@SlaunchaMan SlaunchaMan merged commit 4de1156 into AFNetworking:master Nov 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants