-
-
Notifications
You must be signed in to change notification settings - Fork 612
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
tests(migration): typescript #613
Conversation
Started the migration of other files. Please follow the same pattern |
Can I help in this migration too? |
e89dde5
to
47d70e3
Compare
Hi @rishabh3112, surely you can! There's a task list that I added inside the main issue with the different packages and we are going to tick the ones that are migrated. Please feel free to pick one and tell us, so we are going to update the main thread and we are not going to work on the same thing. Most of them don't require an actual tests because they are just wrappers (add, update, etc.). The main bulk will be Please use the
|
Thanks @ematipico 🙌, |
I am facing a little difficulty in setting up. |
I think this PR should be on hold until the new test infrastructure is up |
Since the new infra is now ready, we can start working on this. Every contribution is welcomed. We will continue working on @rishabh3112 Not sure why GitHub isn't allowing me to assign you. 😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs rebase
@dhruvdutt @evenstensberg I've worked on the pending requested changes and made a PR, would be great to have any kind of feedback 👍 |
tests(migration): including requested changes
Working on rebasing this. |
Status? |
@sendilkumarn Thanks for your update. I labeled the Pull Request so reviewers will review it again. @ematipico Please review the new changes. |
I merged upstream changes. Hopefully, this one will be good to go 🤞 |
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
package.json
Outdated
@@ -43,7 +43,8 @@ | |||
"travis:integration": "npm run build && npm run test && npm run reportCoverage", | |||
"travis:lint": "npm run build && npm run lint && npm run tslint", | |||
"tslint": "tslint -c tslint.json \"packages/**/*.ts\"", | |||
"watch": "npm run build && tsc -w" | |||
"watch": "npm run build && tsc -w", | |||
"test:local": "jest --watch" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we remove this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense 👍
As a final check, could you pull master branch, check if we are missing something? |
This has latest master in it. so we can merge |
@evenstensberg Let us not wait further. I will merge this. I think on the Typescript side it is better to follow some conventions like utilizing type inferences and being strict on any (things like that). I will create a new PR out of this. |
What kind of change does this PR introduce?
Tests: Refactoring to TypeScript
Did you add tests for your changes?
Yes
If relevant, did you update the documentation?
NA
Summary
Added
ts-test
branch on upstream. We will keep working on this branch to migrate all tests to TypeScript.Does this PR introduce a breaking change?
No
Other information
cc @sendilkumarn @ematipico
Packages that contains tests to port to typescript