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

#964 solution #965

Open
wants to merge 1 commit into
base: feature/initJsonString
Choose a base branch
from

Conversation

CasparZheng
Copy link

@CasparZheng CasparZheng commented May 26, 2018

The PR should summarize what was changed and why. Here are some questions to
help you if you're not sure:

  • What behavior was changed?
    compatible with url keyword '#'

  • What code was refactored / updated to support this change?
    url encode characterSet have been changed
    update the url test in StringTests.swift

  • What issues are related to this PR? Or why was this change introduced?
    issue json.url method returned url encoded the '#' #964

Checklist - While not every PR needs it, new features should consider this list:

  • Does this have tests?
    yes
  • Does this have documentation?
    no
  • Does this break the public API (Requires major version bump)?
    no
  • Is this a new feature (Requires minor version bump)?
    no

@@ -39,6 +39,9 @@ class StringTests: XCTestCase {
func testURL() {
let json = JSON("http://github.com")
XCTAssertEqual(json.URL!, URL(string:"http://github.com")!)

Choose a reason for hiding this comment

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

Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)

return Foundation.URL(string: encodedString_)
} else {
return nil
}else {

Choose a reason for hiding this comment

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

Statement Position Violation: Else and catch should be on the same line, one space after the previous declaration. (statement_position)

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.

None yet

2 participants