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

Remove google-libphonenumber and migrate to jest/eslint #573

Closed
wants to merge 2 commits into from
Closed

Remove google-libphonenumber and migrate to jest/eslint #573

wants to merge 2 commits into from

Conversation

rmuchall
Copy link
Contributor

@rmuchall rmuchall commented Apr 3, 2020

Hi there,

Following my PR to routing-controllers (typestack/routing-controllers#553) I'm now submitting a similar PR for class-validator.

Action taken
Removed google-libphonenumber
Reasoning
google-libphonenumber is huge. Although a work-around exists I believe the better option is to remove it. See (#248)

Action taken
Replaced mocha, chai, sinon and nyc with jest
Reasoning
To normalize the test framework across all my projects
Jest appears to be a more complete framework
Jest is more popular
https://npmcompare.com/compare/jest,mocha

Action taken
Replaced gulp with npm scripts (copyfiles, json, rimraf)
Reasoning
Gulp is awesome but we can achieve the same result by using npm scripts (same as routing-controllers)

Action taken
Replaced tslint with eslint
Reasoning
tslint is deprecated and the recommended replacement is eslint

Other changes
Updated all dependencies

Thanks,
Rob

rmuchall added 2 commits April 3, 2020 12:05
Replaced mocha, chai, sinon and nyc with jest
Refactored tests to use jest
Removed google-libphonenumber
Removed gulp and associated dependencies
Replaced gulp with npm scripts (copyfiles, json, rimraf)
Updated all dependencies
@vlapo
Copy link
Contributor

vlapo commented Apr 3, 2020

Hi @rmuchall. Thank you for creating this PR! Unfortunately we have one big PR in review (#568) and we have to wait until merge. But after merging I am open check this PR. For now just few comments from my side:

@rmuchall
Copy link
Contributor Author

rmuchall commented Apr 3, 2020

Hi @vlapo,
Many thanks for your feedback.
I'm happy to modify my PR to meet your requirements.
Thank you for all your hard work on this library! :)

@vlapo
Copy link
Contributor

vlapo commented Apr 18, 2020

Hi @rmuchall.
#568 merged and I would love to see eslint and jest in our code. Could you please create two separate PRs? One for jest and one for eslint.

@rmuchall
Copy link
Contributor Author

Hi @vlapo
No problem. I will close this PR and open 2 new ones.

@rmuchall rmuchall closed this Apr 18, 2020
@github-actions
Copy link

github-actions bot commented Aug 3, 2020

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants