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

Store configuration and ServerPool on the app #78

Merged
merged 1 commit into from
Mar 2, 2020

Conversation

gmacon
Copy link
Collaborator

@gmacon gmacon commented Jan 18, 2020

This is the second PR addressing #40. The configuration is now always read from the current app. While working on this, I realized that the ServerPool is more like configuration that state since it doesn't hold on to open connections. (i.e. it isn't a connection pool). I therefore stored it on the app, too.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 95.472% when pulling 2682858 on gmacon:issue-40-state-on-context into 7f120df on nickw444:master.

@coveralls
Copy link

coveralls commented Jan 18, 2020

Coverage Status

Coverage decreased (-0.4%) to 95.472% when pulling 50e9469 on gmacon:issue-40-state-on-context into 81805de on nickw444:master.

@gmacon
Copy link
Collaborator Author

gmacon commented Feb 16, 2020

I need to rebase this to resolve the conflicts with blackening the code in #81.

@gmacon
Copy link
Collaborator Author

gmacon commented Feb 23, 2020

OK, I've rebased this to resolve the formatting conflicts.

@gmacon gmacon requested a review from nickw444 February 23, 2020 19:37
Copy link
Collaborator

@HeMan HeMan left a comment

Choose a reason for hiding this comment

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

I think you should make flake8 happier.

@gmacon
Copy link
Collaborator Author

gmacon commented Feb 25, 2020

I tripped over psf/black#1202.

Copy link
Collaborator

@HeMan HeMan left a comment

Choose a reason for hiding this comment

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

I think it's ok to merge even with decreased coverage.

@gmacon gmacon merged commit 9b1f850 into nickw444:master Mar 2, 2020
@gmacon gmacon deleted the issue-40-state-on-context branch March 2, 2020 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants