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

Fixes #1293 - Add tests for clay-charts-react #1294

Merged
merged 1 commit into from
Nov 14, 2018

Conversation

bryceosterhaus
Copy link
Member

There were a bunch of issues getting jest/yarn/lerna to work with different versions of babel so I ended up just falling back to the same babel version for all the packages. This allowed em to write some basic render tests for clay-charts-react

@bryceosterhaus bryceosterhaus force-pushed the develop branch 3 times, most recently from 67698cd to 64e65df Compare November 8, 2018 00:22
@matuzalemsteles
Copy link
Member

hey @bryceosterhaus, I think we can try to update the babel version to 7.x for all packages and see what happens. We have to analyze this, I had seen some cases where the babel transformed some code to specifications with use of Symbol for example, forcing us to use a polyfill for this 🤷‍♂️.

@matuzalemsteles
Copy link
Member

Just started reviewing :)

:octocat: Sent from GH.

Copy link
Member

@matuzalemsteles matuzalemsteles left a comment

Choose a reason for hiding this comment

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

LGTM!

@matuzalemsteles
Copy link
Member

hey @bryceosterhaus, It would be something interesting then we think of a way to create tests to ensure the consistency of APIs that are passed to the charts in react and metal.

@jbalsas
Copy link
Contributor

jbalsas commented Nov 13, 2018

Hey @bryceosterhaus, I think this is conflicting with our fix for Node 10 from yesterday. Can you resolve those and push so we can merge?

This is looking cool! 👏

@bryceosterhaus
Copy link
Member Author

Updated!

@jbalsas jbalsas merged commit 37e1473 into liferay:develop Nov 14, 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.

3 participants