-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
Update documentation. #113
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.
Looking good 👍 just a few thoughts
Documentation/Basics.md
Outdated
# Basic Usage # | ||
|
||
Let's say you have an endpoint, `GET https://www.example.com/comments/1`, that returns the following JSON: | ||
Let's say we have an endpoint, `https://raw.githubusercontent.com/thoughtbot/Swish/master/Documentation/example.json` |
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.
Should we leave GET
in here?
Documentation/Basics.md
Outdated
"commentText": "Pretty good. Pret-ty pre-ty pre-ty good.", | ||
"username": "LarryDavid" | ||
} | ||
``` | ||
|
||
We'll model this with the following struct, and implement Argo's `Decodable` protocol to tell it how to deal with JSON: | ||
We will model the data with the following struct, | ||
and implement the `Codable` protocol so we can |
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.
implement -> conform to
Documentation/Basics.md
Outdated
} | ||
``` | ||
|
||
We can then use the Swish's default `APIClient` to make the request: | ||
We can then use the Swish's default APIClient to make the request: |
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.
Do you think it makes sense to leave the backticks in here?
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.
Yep!
Documentation/Basics.md
Outdated
func build() -> URLRequest { | ||
let url = URL(string: "https://raw.githubusercontent.com/thoughtbot/Swish/master/Documentation/example.json")! | ||
return URLRequest(url: url) | ||
} |
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.
Do you think it's necessary to reindent these examples?
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.
Nope!
README.md
Outdated
| 2.X | 1.X | | ||
| Swift Version | Swish Version | | ||
| ------------- | ------------ | | ||
| 4.X | >= 2.0.3 | |
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.
Should we bump the major version for this change?
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.
Definitely!
|
||
PlaygroundPage.current.needsIndefiniteExecution = true | ||
|
||
// Let's say we have an endpoint, https://raw.githubusercontent.com/thoughtbot/Swish/master/Documentation/example.json |
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.
Would it be possible to use playground markup instead of regular comments? Or does it automatically render these as Markdown?
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.
Good idea 👍
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.
Couple more comments, but
Documentation/Basics.md
Outdated
<*> j <| "commentText" | ||
<*> j <| "username" | ||
enum CodingKeys: String, CodingKey { | ||
case id = "id" |
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.
FWIW, you can leave off the explicit = "id"
if the string value is the same as the case name. (Same goes for username
below.)
README.md
Outdated
|
||
You can see an example of Swish in action via the included `SwishExamples.playground`. | ||
|
||
To use that, clone this repository and run `carthage bootstrap`. When that |
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.
Carthage used to build for all platforms by default — if this is still the case, could we add --platform iOS
to match the platform the playground targets?
struct CommentRequest: Request { | ||
typealias ResponseObject = Comment | ||
|
||
|
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.
Extra newline
- Documentation/Basics.md - Added SwishExamples.playground playground file, with the contents of Basics.md
f064fff
to
8c4757f
Compare
Documentation/Basics.md
to include the removal of Argo, and working examples viaCodable
.SwishExamples.playground
that contains the code inDocumentation/Basics.md
.README.md
with instructions on how to use it.If you checkout this branch, the URL in
SwishExamples.playground
needs to be set tohttps://raw.githubusercontent.com/thoughtbot/Swish/sr-update-documentation/Documentation/example.json
for the example to work; the URL that is in there now will work once this branch is merged to master.