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

Add product evaluation to engine and view renderer. Closes #705. #812

Merged
merged 3 commits into from
Feb 23, 2018

Conversation

chambridge
Copy link
Contributor

No description provided.

@chambridge chambridge requested review from a team February 23, 2018 04:31
@coveralls
Copy link

coveralls commented Feb 23, 2018

Coverage Status

Coverage decreased (-0.2%) to 91.224% when pulling c60f699 on issues/705-2 into f9c738f on master.

Copy link
Contributor

@kholdaway kholdaway left a comment

Choose a reason for hiding this comment

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

Approved. Couple of minor python comments.

for fact_key in fact.keys():
headers.add(fact_key)
if fact_key == 'products':
prods = fact.get(fact_key)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can prods ever be be None?

priority_prod_dict[prod[NAME_KEY]] = prod
elif merge_prod is None:
priority_prod_dict[prod[NAME_KEY]] = prod
for _, prod in priority_prod_dict:
Copy link
Contributor

Choose a reason for hiding this comment

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

priority_prod_dict.values()?

@chambridge chambridge merged commit 0120f6c into master Feb 23, 2018
@chambridge chambridge deleted the issues/705-2 branch February 23, 2018 15:16
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.

3 participants