Skip to content

Migrate to jest/eslint #338

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

Merged
merged 2 commits into from
Jul 20, 2020
Merged

Migrate to jest/eslint #338

merged 2 commits into from
Jul 20, 2020

Conversation

rmuchall
Copy link
Contributor

@rmuchall rmuchall commented Apr 3, 2020

Hi folks,

Following my other PRs to typestack here is a similar one for class-transformer.
typestack/routing-controllers#553
typestack/class-validator#573

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 tslint with eslint
Reasoning
tslint is deprecated and the recommended replacement is eslint

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)

Thanks,
Rob

rmuchall added 2 commits April 3, 2020 15:22
Replaced mocha, chai, sinon and nyc with jest
Refactored tests to use jest
Removed gulp and associated dependencies
Replaced gulp with npm scripts (copyfiles, json, rimraf)
Updated all dependencies
@rmuchall rmuchall changed the title Migrate to Jest/eslint Migrate to jest/eslint Apr 3, 2020
@jotamorais jotamorais self-assigned this Jun 29, 2020
@jotamorais jotamorais added this to the Future milestone Jun 29, 2020
@jotamorais
Copy link
Member

@NoNameProvided, I reviewed this PR in preparation to integrate other PRs and other important fixes (such as #339)

I cloned and integrated it locally but when I tried and to push to the develop branch, I noticed that the branch is protected and and the CI won't allow me to push. Would you please either add your review to the PR as well, or grant me permission to push to that branch (or remove that status check from Travis?

Here's what I get:

image

Thanks!

@jotamorais
Copy link
Member

@NoNameProvided, I reviewed this PR in preparation to integrate other PRs and other important fixes (such as #339)

I cloned and integrated it locally but when I tried and to push to the develop branch, I noticed that the branch is protected and and the CI won't allow me to push. Would you please either add your review to the PR as well, or grant me permission to push to that branch (or remove that status check from Travis?

Here's what I get:

image

Thanks!

@NoNameProvided / @pleerock
Appreciate if you could help me with this.

@jotamorais
Copy link
Member

@jotamorais, I was talking with @pleerock. Could you try again?

@saulotoledo & @pleerock,

I tried again but the branch is still protected and with the commit hook and won't allow me to push.

image

"clean": "rimraf build coverage",
"copy": "copyfiles -u 3 \"build/compiled/src/**/*\" build/package && copyfiles package.json README.md build/package",
"lint": "eslint --config ./.eslintrc.js --ext .ts ./src ./test",
"package": "npm run build && npm run copy && npm run public && rimraf build/compiled",
Copy link
Member

@saulotoledo saulotoledo Jul 20, 2020

Choose a reason for hiding this comment

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

I suggest to use https://github.com/mysticatea/npm-run-all here to make it cleaner. (It does not need to be done in this PR.)

@saulotoledo
Copy link
Member

@jotamorais Please find me on gitter if you have some time later.

@pleerock
Copy link
Contributor

pleerock commented Jul 20, 2020

@jotamorais I don't watch for notifications in these repositories, since I don't maintain them. But you can reach me in email or typeorm's slack in case if you need something from me. @NoNameProvided I temporary switched number of approvals to master back to 1 since it looks like there are few very important PRs here that people wants to get merged. Sorry if I did something wrong, you can return it back to 2 once you have a time.

@jotamorais jotamorais self-requested a review July 20, 2020 20:38
@jotamorais jotamorais merged commit 94db7dc into typestack:master Jul 20, 2020
@rmuchall rmuchall deleted the jest-eslint branch July 20, 2020 20:51
@github-actions
Copy link

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 20, 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.

4 participants