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

docs: minimal application example #8

Merged
merged 1 commit into from
Sep 2, 2014

Conversation

jirikuncar
Copy link
Member

Signed-off-by: Jiri Kuncar jiri.kuncar@gmail.com

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling c3ee9e8 on jirikuncar:docs into 74e1b97 on inveniosoftware:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 9db8770 on jirikuncar:docs into 74e1b97 on inveniosoftware:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 3839c52 on jirikuncar:docs into 74e1b97 on inveniosoftware:master.

@tiborsimko
Copy link
Member

Thanks, the example looks good, here are some cosmetic comments to the presentation:

  • in the auto-generated API, the module help and the SSO class help are basically saying the same thing and they are presented right after each other, which looks kind of weird in the generated HTML. Suggestion: cut down the module docstring description so that it does not repeat the same thing that the class docstring says. (or vice versa)
  • In the usage example, introduce trailing colons, and say things in a guided explicit order, like:
    • "First, let's create the application and initialise the extension:"
    • "Second, let's configure the attribute map for converting environment variables to a dictionary containing user information:"
    • "Third, let's set up a login handler function that reads user information and stores it for later usage:"
    • "Fourth, we can now greet the user using his SSO login name:"

Reviewed-by: Tibor Simko <tibor.simko@cern.ch>
Signed-off-by: Jiri Kuncar <jiri.kuncar@cern.ch>
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 6262a89 on jirikuncar:docs into 74e1b97 on inveniosoftware:master.

@tiborsimko tiborsimko merged commit 6262a89 into inveniosoftware:master Sep 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants