-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
fix(jasmine): remove unused options param #10240
Conversation
Thanks! I added it to the milestone so we don't forget 👍 |
Test failures are not related to this change :) |
You sure? Master is green (except for flaky mac and windows on node 14). EDIT: Wait, network stuff. I restarted one of them, but will be merging in master when this is ready to land |
Yeah, because all I did was rebase and if a PR came along that used the parameter, it would have exploded TypeScript :) |
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.
Thanks! Could you rebase and add a changelog entry?
Codecov Report
@@ Coverage Diff @@
## master #10240 +/- ##
=======================================
Coverage 64.24% 64.24%
=======================================
Files 308 308
Lines 13502 13502
Branches 3289 3289
=======================================
Hits 8675 8675
Misses 4117 4117
Partials 710 710
Continue to review full report at Codecov.
|
🚀 |
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. |
Summary
Per #10216 (comment), I discovered that this parameter isn't actually used, and so ideally should be removed in Jest 27.
(this PR is based on the aforementioned PR, to skip having to deal with merge conflicts - I've also not made a changelog entry for the same reason).
Test plan
Run the tests, see what breaks.