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

Port displayPrice resolver from 3.0.0 branch #6046

Closed
wants to merge 2 commits into from

Conversation

chrispotter
Copy link

Signed-off-by: Chris Potter chris@reactioncommerce.com

Impact: minor
Type: feature

Issue

Ports working resolver and test from 3.0.0 to 2.9.

Signed-off-by: Chris Potter <chris@reactioncommerce.com>
@chrispotter
Copy link
Author

not sure what's up with the tests here, any suggestions @aldeed or @focusaurus?

@focusaurus
Copy link
Contributor

Hmm, the sharp npm module is notoriously difficult to reliably build. The errors are not introduced by your change though. Let my try circleci with ssh and see if I can figure out what's going on.

@kieckhafer
Copy link
Member

@chrispotter i was running into the same issue in #6049 , I updated sharp in the package.json to the latest release which came out a bout a week ago, and that seemed to fix it. We are still seeing what looks like some failures in the meteor npm install step, although it still passed CI so not 100% sure what the deal is. Try updating sharp to ^0.24.0 and see if that helps.

Signed-off-by: Chris Potter <chris@reactioncommerce.com>
@focusaurus
Copy link
Contributor

Note as I understand our current requirements, this is not technically required in 2.x for us. I think it's pretty harmless and OK to merge nonetheless, but if anyone has reason to leave this in 3.x only that's fine too AFAIK.

@aldeed
Copy link
Contributor

aldeed commented Jan 29, 2020

@focusaurus @chrispotter Yeah I don't see why we'd need this in 2.x. Is it fixing any issue? Seems like it's only related to new catalog streaming that is 3.0 only. If so, I would vote to not merge since 2.x is supposed to be getting urgent fixes only.

@chrispotter
Copy link
Author

this was to deploy for testing purposes against an internal environment running 2.9, but since we've decided to upgrade this environment to 3.0.0-beta I think it's better to close. 👍

@chrispotter chrispotter deleted the chrispotter-displayprice-resolver branch January 29, 2020 16: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.

None yet

4 participants