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 the support for chai-should/expect #42

Closed
wants to merge 22 commits into from

Conversation

benjamin99
Copy link

No description provided.

@benjamin99
Copy link
Author

Hi,
just trying to add the transformers for chai (should & test) apis. Hope this PR can help the project.

@skovhus
Copy link
Owner

skovhus commented Mar 24, 2017

Hi! Thanks for doing this.

There is already a fairly feature complete codemod for this, AlexJuarez/chai-to-jasmine#1, that we have been talking about integrating (there is an issue for that).

Did you have a look on that one? If not I think it makes sense to do that. Just copy paste the transformer and modify it to match our needs.

Please ensure that you support all chai should methods, if something is not working there should be a warning.

Again, thanks for taking your time doing this. : )

@benjamin99
Copy link
Author

Sure I would be more than willing to do that.
Would it be okey for you @AlexJuarez ?

@skovhus
Copy link
Owner

skovhus commented Mar 24, 2017

Seems so AlexJuarez/chai-to-jasmine#1

@AlexJuarez
Copy link

If you could cleanup the chai-to-jamine transform @benjamin99 found here https://github.com/AlexJuarez/chai-to-jasmine/blob/master/transforms/chai-to-jasmine.js you could also copy in all of the tests https://github.com/AlexJuarez/chai-to-jasmine/tree/master/transforms/__testfixtures__/chai-to-jasmine.

Or if you want me to do that I could finish up my original PR. again I had stopped because the approaches are fairly different, and I wasn't sure how much @skovhus cared about conforming to the code style.

@skovhus
Copy link
Owner

skovhus commented Mar 27, 2017

As long as we have a good test coverage and it works, I really don't have hard feeling around style. Eslint --fix does most of the work. : )

@AlexJuarez
Copy link

@benjamin99 @skovhus I made a PR #43 see if that works for your needs. Thanks

@skovhus
Copy link
Owner

skovhus commented Apr 1, 2017

@benjamin99 thank you for your work here. I'll move the CLI work of this over to https://github.com/skovhus/jest-codemods/tree/AlexJuarez-chai-to-jasmine

Let me know if you would like to contribute to that branch. Maybe some of your tests cases can be moved over.

Thanks!

@skovhus skovhus closed this Apr 1, 2017
@skovhus skovhus mentioned this pull request Apr 1, 2017
6 tasks
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