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

Add usage example to APIDoc and updated README #51

Merged
merged 5 commits into from
Feb 13, 2019

Conversation

helenmasters
Copy link
Contributor

  • Added usage example to APIDoc for Session and CookieParameters
  • Added further descriptions to parameters in APIDoc
  • Fixed typos in APIDoc
  • Added APIDoc link to the README header
  • Added missing standard sections to README
  • Reworded parts of the README

@helenmasters helenmasters added the jazzy-doc When applied to a PR, instructs Package-Builder to regenerate the Jazzy documentation label Jul 11, 2018
@helenmasters helenmasters requested a review from djones6 July 11, 2018 13:33
Copy link
Contributor

@djones6 djones6 left a comment

Choose a reason for hiding this comment

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

Good fixes, I feel I'm nitpicking here but some wording suggestions.

README.md Outdated
import KituraSession
import KituraSessionRedis

let redisStore = RedisStore(redisHost: host, redisPort: port)
let session = Session(secret: "Some secret", store: redisStore)
router.all(middleware: session)
```
First an instance of `RedisStore` is created (see [`KituraSessionRedis`](https://github.com/IBM-Swift/Kitura-Session-Redis) for more information), then an instance of `Session` with the store as parameter is created, and finally it is connected to the desired path.
First an instance of `RedisStore` is created (see [`KituraSessionRedis`](https://github.com/IBM-Swift/Kitura-Session-Redis) for more information), then an instance of `Session` with the store as a parameter is created, then it is connected to the desired path.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer

An instance of RedisStore is created that will be used to persist session data (see KituraSessionRedis for more information). An instance of Session is then created, specifying redisStore as the session store. Finally, the session instance is registered as a middleware on the desired path.

case name(String)

/// The cookie's Path attribute.
/// The cookie's path attribute, this defines the path for which this cookie should be supplied.
Copy link
Contributor

Choose a reason for hiding this comment

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

The cookie's path attribute. This specifies the path for which the cookie is valid. The client should only provide this cookie for requests on this path.

public enum CookieParameter {

/// The cookie's name.
/// The cookie's name, defaults to "kitura-session-id".
Copy link
Contributor

Choose a reason for hiding this comment

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

name, defaults to -> name. Defaults to

case path(String)

/// The cookie's Secure attribute.
/// The cookie's secure attribute, this indicates whether the cookie should be provided only
Copy link
Contributor

Choose a reason for hiding this comment

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

attribute, this indicates whether -> attribute, indicating whether

helenmasters and others added 2 commits January 30, 2019 14:36
 * Added usage example to APIDoc for Session and CookieParameters
 * Added further descriptions to parameters in APIDoc
 * Fixed typos in APIDoc
 * Added APIDoc link to the README header
 * Added missing standard sections to README
 * Reworded parts of the README
@Andrew-Lees11
Copy link
Contributor

I have addressed mine and @djones6 review comments and rebased this PR on top of the current master.
I also added readme examples for Codable and TypeSafeSession to the README.

@Andrew-Lees11 Andrew-Lees11 merged commit bb7b535 into master Feb 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jazzy-doc When applied to a PR, instructs Package-Builder to regenerate the Jazzy documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants