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

Document params of Application.make_handler(). #1132

Merged
merged 1 commit into from
Aug 28, 2016

Conversation

f0t0n
Copy link
Contributor

@f0t0n f0t0n commented Aug 27, 2016

What do these changes do?

Explicitly document parameters of aiohttp.web.Application.make_handler() method.

Are there changes in behavior for the user?

Documentation became more clear.

Related issue number

#1129

Also see this thread: #1123 (comment)

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes

@codecov-io
Copy link

codecov-io commented Aug 27, 2016

Current coverage is 98.28% (diff: 100%)

Merging #1132 into master will not change coverage

@@             master      #1132   diff @@
==========================================
  Files            28         28          
  Lines          6465       6465          
  Methods           0          0          
  Messages          0          0          
  Branches       1083       1083          
==========================================
  Hits           6354       6354          
  Misses           61         61          
  Partials         50         50          

Powered by Codecov. Last update dac1333...9486c52

@f0t0n
Copy link
Contributor Author

f0t0n commented Aug 27, 2016

@asvetlov Please check descriptions of parameters. I didn't find proper descriptions for some of them. Maybe you could point how to describe better. Also maybe we should drop some from the list.
At the moment on the built page they look like this:

image

'0.0.0.0', 8080)
Creates HTTP protocol factory for handling requests.

:param handler: Request handler. Default:
Copy link
Member

Choose a reason for hiding this comment

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

Let's drop it from doc.
It's a hidden param, not part of public API (at least now now).

@f0t0n f0t0n force-pushed the feature/doc-app-make-handler branch from beb8af8 to 5736198 Compare August 27, 2016 23:38
@f0t0n
Copy link
Contributor Author

f0t0n commented Aug 27, 2016

All the changes according to the comments in code review are done.

- Dropped hidden parameters, and the code example for
     `secure_proxy_ssl_header` according to code review notes.
@f0t0n f0t0n force-pushed the feature/doc-app-make-handler branch from 5736198 to 9486c52 Compare August 27, 2016 23:48
@asvetlov asvetlov merged commit 23aaaf5 into aio-libs:master Aug 28, 2016
@asvetlov
Copy link
Member

Thanks

@f0t0n f0t0n deleted the feature/doc-app-make-handler branch August 28, 2016 20:14
@lock
Copy link

lock bot commented Oct 29, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants