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

improve basic usage in pool document #347

Closed
wants to merge 1 commit into from
Closed

Conversation

crvv
Copy link
Contributor

@crvv crvv commented Oct 18, 2018

This example has some issues:

  1. with (yield from pool) as conn has been deprecated in Replace yield from with await... #283
  2. The cursor is not closed in the example. It uses context manager for conn but not for cursor.
  3. The indent of line 28 is wrong.

Besides, the basic usage of pool is in the README.rst of the project.
I don't think one more example is valuable.

@codecov
Copy link

codecov bot commented Oct 18, 2018

Codecov Report

Merging #347 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #347   +/-   ##
=======================================
  Coverage   92.28%   92.28%           
=======================================
  Files           9        9           
  Lines        1180     1180           
  Branches      172      172           
=======================================
  Hits         1089     1089           
  Misses         64       64           
  Partials       27       27

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 131fb9f...9e3c4aa. Read the comment docs.

@jettify
Copy link
Member

jettify commented Dec 1, 2018

Sort of agree, but I think good idea to rework example, instead of deleting it, more examples the better.

@crvv crvv changed the title remove basic usage in pool document improve basic usage in pool document Mar 5, 2019
@Nothing4You
Copy link
Collaborator

thanks for the PR.

the incorrect indent was already fixed in #418.
I plan to remove the yield syntax with #582, which does not only cover this pool example.
I've created #665 to track closing the cursor in the example properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants