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

Quote Engine Non-CDN Retrieval #48

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

wazaundtechnik
Copy link
Contributor

Too tired today... still WIP. I will update this later.

Update: Done everything. Finally!

@wazaundtechnik wazaundtechnik changed the title WIP Quote Engine Non-CDN Retrieval Quote Engine Non-CDN Retrieval Oct 31, 2015
@kmkurn
Copy link

kmkurn commented Jan 9, 2016

@wazaundtechnik since you said using raw Github is enough for our usage, is this PR still relevant?

@wazaundtechnik
Copy link
Contributor Author

wazaundtechnik commented Jan 9, 2016 via email

@kmkurn
Copy link

kmkurn commented Jan 9, 2016

@wazaundtechnik wow that's cool. okay then. can @fushar take a look at this PR too? now that we have another user (beside our own group) maybe it's time to work on this again hehe.



@pytest.fixture
def qe():
Copy link

Choose a reason for hiding this comment

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

can you please use a more descriptive name? it's not obvious what qe stands for.

Copy link

Choose a reason for hiding this comment

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

before that, why did you put this as fixture? as I understand it, this is your system under test. shouldn't you explicitly instantiate QuoteEngine inside your test case and not provide this as fixture? this is not going to be used anywhere else (at least for now).

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.

2 participants