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

Product options #550

Merged
merged 6 commits into from
Nov 27, 2018
Merged

Product options #550

merged 6 commits into from
Nov 27, 2018

Conversation

jimbo
Copy link
Contributor

@jimbo jimbo commented Nov 27, 2018

This PR is a:

  • New feature
  • Enhancement/Optimization
  • Refactor
  • Bugfix
  • Test for existing code
  • Documentation

Summary

When this pull request is merged, it will enable shoppers to select values for configurable options and purchase configurable products.

Additional information

@coveralls
Copy link

coveralls commented Nov 27, 2018

Coverage Status

Coverage decreased (-0.3%) to 17.428% when pulling d1cd5d6 on jimbo/product-options into 3f3a48a on release/2.0.

@jimbo jimbo requested a review from zetlen November 27, 2018 18:06
Copy link
Contributor

@zetlen zetlen left a comment

Choose a reason for hiding this comment

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

This code is high quality and basically ready for merge. Thank you, @pcvonz and @jimbo, for working so hard on it.

I shoot it back as "request changes" only because of one or two small tweaks, and a general need for comments in the areas where we're doing something weird. I can add these comments if necessary before merge, but you can if you prefer.

@pcvonz
Copy link
Contributor

pcvonz commented Nov 27, 2018

Storybook is missing again. But I think that can be handled in a separate issue.
Edit: separate issue 🎊

@zetlen zetlen merged commit 9a65584 into release/2.0 Nov 27, 2018
@jimbo jimbo deleted the jimbo/product-options branch November 27, 2018 22:55
@jimbo jimbo mentioned this pull request Nov 27, 2018
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.

4 participants