Skip to content

[jasmine] Allow to use argument Matchers in withArgs()#38738

Merged
armanio123 merged 1 commit intoDefinitelyTyped:masterfrom
zvirja:allow-to-use-matchers-in-withargs
Oct 10, 2019
Merged

[jasmine] Allow to use argument Matchers in withArgs()#38738
armanio123 merged 1 commit intoDefinitelyTyped:masterfrom
zvirja:allow-to-use-matchers-in-withargs

Conversation

@zvirja
Copy link
Copy Markdown
Contributor

@zvirja zvirja commented Sep 30, 2019

Please fill in this template.

  • Use a meaningful title for the pull request. Include the name of the package modified.
  • Test the change in your own code. (Compile and run.)
  • Add or edit tests to reflect the change. (Run with npm test.)
  • Follow the advice from the readme.
  • Avoid common mistakes.
  • Run npm run lint package-name (or tsc if no tslint.json is present).

If changing an existing definition:

  • Provide a URL to documentation or source code which provides context for the suggested changes: see specs from jasmine library.
  • If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the header.
  • If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }. If for reason the any rule need to be disabled, disable it for that line using // tslint:disable-next-line [ruleName] and not for whole package so that the need for disabling can be reviewed.

Allow to use argument matchers in withArgs() util.

@zvirja zvirja requested a review from borisyankov as a code owner September 30, 2019 21:22
@typescript-bot typescript-bot added Where is Travis? Popular package This PR affects a popular package (as counted by NPM download counts). Author is Owner The author of this PR is a listed owner of the package. labels Sep 30, 2019
@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Sep 30, 2019 •
edited
Loading

@zvirja Thank you for submitting this PR!

🔔 @borisyankov @theodorejb @davidparsson @gmoothart @lukas-zech-software @Engineer2B @cyungmann @Roaders @devoto13 @FDIM @pe8ter @kolodny @stephenfarrar @ndunks - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Sep 30, 2019

Since you're a listed owner and the build passed, this PR is fast-tracked. A maintainer will merge shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped!

@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Sep 30, 2019

@zvirja - It appears Travis did not correctly run on this PR! /cc @RyanCavanaugh to investigate and advise.

@zvirja zvirja mentioned this pull request Sep 30, 2019
Merged
6 tasks
@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Oct 8, 2019

👋 Hi there! I’ve run some quick performance metrics against master and your PR. This is still an experiment, so don’t panic if I say something crazy! I’m still learning how to interpret these metrics.

Let’s review the numbers, shall we?

Comparison details 📊
master #38738 diff
Batch compilation
Memory usage (MiB) 86.0 77.1 -10.4%
Type count 12899 12905 0.0%
Assignability cache size 35146 35147 0.0%
Subtype cache size 129 129 0.0%
Identity cache size 40 40 0.0%
Language service
Samples taken 2210 2229 +0.9%
Identifiers in tests 2210 2229 +0.9%
getCompletionsAtPosition
    Mean duration (ms) 424.8 414.6 -2.4%
    Median duration (ms) 422.6 411.4 -2.6%
    Mean CV 12.9% 13.3% +3.4%
    Worst duration (ms) 557.9 579.1 +3.8%
    Worst identifier expect onload
getQuickInfoAtPosition
    Mean duration (ms) 409.5 400.0 -2.3%
    Median duration (ms) 407.7 398.1 -2.3%
    Mean CV 12.8% 13.3% +3.9%
    Worst duration (ms) 593.0 518.1 -12.6%
    Worst identifier createSpyObj any

It looks like nothing changed too much. I’m pretty lenient since I’m still an experiment, so take a look anyways and make sure nothing looks out of place.

If you have any questions or comments about me, you can ping @andrewbranch. Have a nice day!

@armanio123
Copy link
Copy Markdown

armanio123 commented Oct 8, 2019

Reviewers, I appreciate your help taking a look a t this PR.

I'll wait a couple more days for a review, otherwise I'll merge this as is.

@armanio123 armanio123 merged commit aed6042 into DefinitelyTyped:master Oct 10, 2019
@zvirja zvirja deleted the allow-to-use-matchers-in-withargs branch October 10, 2019 20:00
@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Oct 10, 2019

I just published @types/[email protected] to npm.

alexanderson1993 pushed a commit to alexanderson1993/DefinitelyTyped that referenced this pull request Oct 11, 2019
@zvirja
Allow to use argument matchers in withArgs() (DefinitelyTyped#38738)
1fbca6a
chivesrs pushed a commit to chivesrs/DefinitelyTyped that referenced this pull request Nov 19, 2019
@zvirja @chivesrs
Allow to use argument matchers in withArgs() (DefinitelyTyped#38738)
a4b5461
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@borisyankov borisyankov Awaiting requested review from borisyankov borisyankov is a code owner

Assignees

No one assigned

Labels

Author is Owner The author of this PR is a listed owner of the package. Popular package This PR affects a popular package (as counted by NPM download counts).

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zvirja @typescript-bot @armanio123