[@types/underscore] Collection and Array Tests - Without, Uniq, and SortedIndex by reubenrybnik · Pull Request #46068 · DefinitelyTyped/DefinitelyTyped

reubenrybnik · 2020-07-14T06:03:57Z

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).
Provide a URL to documentation or source code which provides context for the suggested changes: https://underscorejs.org/
Include tests for your changes

This PR continues the planned set that will together add up to #45201 and includes the following changes:

Adds tests for without, uniq, and sortedIndex.
Updates uniq and sortedIndex to use Iteratee.
Replaces all unique overloads with a reference to uniq overloads.
Updates the return type of _Chain.uniq to use the correct wrapped value type V to partially fix @types/underscore error TS2322 for _.chain after upgrade to v1.9 #36308.

…ng tests.

typescript-bot · 2020-07-14T06:04:01Z

@reubenrybnik Thank you for submitting this PR!

This is a live comment which I will keep updated.

Code Reviews

Because you edited one package and updated the tests (👏), I can help you merge this PR once someone else signs off on it.

Status

✅ No merge conflicts
✅ Continuous integration tests have passed
✅ Most recent commit is approved by type definition owners or DT maintainers

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 46068,
  "author": "reubenrybnik",
  "owners": [
    "borisyankov",
    "jbaldwin",
    "ccurrens",
    "confususs",
    "jgonggrijp",
    "ffflorian",
    "regevbr",
    "peterblazejewicz",
    "reubenrybnik"
  ],
  "dangerLevel": "ScopedAndTested",
  "headCommitAbbrOid": "1ef7c58",
  "headCommitOid": "1ef7c589e1bbefbc057d10451a5b47995ac205d0",
  "mergeIsRequested": true,
  "stalenessInDays": 0,
  "lastPushDate": "2020-07-15T08:59:48.000Z",
  "lastCommentDate": "2020-07-15T15:33:27.000Z",
  "maintainerBlessed": false,
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/46068/files",
  "hasMergeConflict": false,
  "authorIsOwner": true,
  "isFirstContribution": false,
  "popularityLevel": "Popular",
  "anyPackageIsNew": false,
  "packages": [
    "underscore"
  ],
  "files": [
    {
      "path": "types/underscore/index.d.ts",
      "kind": "definition",
      "package": "underscore"
    },
    {
      "path": "types/underscore/underscore-tests.ts",
      "kind": "test",
      "package": "underscore"
    }
  ],
  "hasDismissedReview": false,
  "ciResult": "pass",
  "lastReviewDate": "2020-07-15T15:20:38.000Z",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 2,
  "isChangesRequested": false
}

typescript-bot · 2020-07-14T06:04:02Z

🔔 @borisyankov @jbaldwin @ccurrens @confususs @jgonggrijp @ffflorian @regevbr @peterblazejewicz — 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.

reubenrybnik · 2020-07-14T06:08:14Z

I thought it would be nice to start in on arrays a bit in parallel with collections. I'll keep it down to only having at most two open PRs at any given time, though.

types/underscore/index.d.ts

typescript-bot · 2020-07-14T07:59:21Z

👋 Hi there! I’ve run some quick measurements against master and your PR. These metrics should help the humans reviewing this PR gauge whether it might negatively affect compile times or editor responsiveness for users who install these typings.

Let’s review the numbers, shall we?

Comparison details 📊

	master	#46068	diff
Batch compilation
Memory usage (MiB)	56.3	58.5	+3.9%
Type count	9635	9948	+3%
Assignability cache size	1493	1493	0%

Language service
Samples taken	4106	4505	+10%
Identifiers in tests	4106	4505	+10%
`getCompletionsAtPosition`
Mean duration (ms)	165.8	149.5	-9.8%
Mean CV	13.9%	14.0%
Worst duration (ms)	314.7	291.2	-7.5%
Worst identifier	_	_
`getQuickInfoAtPosition`
Mean duration (ms)	159.3	144.0	-9.6%
Mean CV	14.1%	14.3%	+1.4%
Worst duration (ms)	333.3	292.4	-12.2%
Worst identifier	extractUnderscoreTypes	extractChainTypes

It looks like nothing changed too much. I won’t post performance data again unless it gets worse.

jgonggrijp

I'm fine with you submitting two PRs in parallel, as long as you keep in mind that I'll be reviewing them serially. ;-)

I'm fine with the changes as they are, although I think it would make sense to add some matcher tests for uniq and sortedIndex.

The typings look perfectly correct to me. 👍

types/underscore/index.d.ts

types/underscore/underscore-tests.ts

…aracters long and adding a lowest index distinction to sortedIndex summary comments.

typescript-bot · 2020-07-15T09:03:03Z

@jgonggrijp Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

reubenrybnik · 2020-07-15T09:21:58Z

I'm done making updates here @jgonggrijp if you wouldn't mind taking one more look and (hopefully) reapproving whenever you get a chance. Thanks for the feedback!

These changes don't result in any new errors in my company's solution (and they fix several errors by updating _Chain.uniq to use the correct wrapped result type since we have several chains that end in a uniq call).

jgonggrijp

Nice work!

reubenrybnik · 2020-07-15T15:33:27Z

Ready to merge

@reubenrybnik

…rray Tests - Without, Uniq, and SortedIndex by @reubenrybnik * Updating type definitions for without, uniq, and sortedIndex and adding tests. * Updating summary comments for updated functions to be less than 80 characters long and adding a lowest index distinction to sortedIndex summary comments. * Adding matcher iteratee tests for uniq and sortedIndex.

Updating type definitions for without, uniq, and sortedIndex and addi…

a90c06e

…ng tests.

reubenrybnik requested a review from borisyankov as a code owner July 14, 2020 06:03

typescript-bot added Where is GH Actions? GH Actions didn't give a response to this PR 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 Jul 14, 2020

typescript-bot removed the Where is GH Actions? GH Actions didn't give a response to this PR label Jul 14, 2020

reubenrybnik commented Jul 14, 2020

View reviewed changes

types/underscore/index.d.ts Show resolved Hide resolved

typescript-bot added the Perf: Same typescript-bot determined that this PR will not significantly impact compilation performance. label Jul 14, 2020

jgonggrijp approved these changes Jul 14, 2020

View reviewed changes

types/underscore/index.d.ts Outdated Show resolved Hide resolved

types/underscore/index.d.ts Show resolved Hide resolved

types/underscore/index.d.ts Outdated Show resolved Hide resolved

types/underscore/underscore-tests.ts Show resolved Hide resolved

reubenrybnik added 2 commits July 15, 2020 01:58

Updating summary comments for updated functions to be less than 80 ch…

5064589

…aracters long and adding a lowest index distinction to sortedIndex summary comments.

Adding matcher iteratee tests for uniq and sortedIndex.

1ef7c58

jgonggrijp approved these changes Jul 15, 2020

View reviewed changes

typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. Self Merge This PR can now be self-merged by the PR author or an owner labels Jul 15, 2020

typescript-bot merged commit 10daadd into DefinitelyTyped:master Jul 15, 2020

This was referenced Jul 19, 2020

[@types/underscore] More comprehensive tests for Underscore collection and array functions + definition changes as appropriate #45201

Closed

[@types/underscore] Collection and Array Tests - Max, Min, and Size #46240

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[@types/underscore] Collection and Array Tests - Without, Uniq, and SortedIndex#46068

[@types/underscore] Collection and Array Tests - Without, Uniq, and SortedIndex#46068
typescript-bot merged 3 commits intoDefinitelyTyped:masterfrom
reubenrybnik:underscore-tests-without-uniq-sortedIndex

reubenrybnik commented Jul 14, 2020 •

edited

Loading

Uh oh!

typescript-bot commented Jul 14, 2020 •

edited

Loading

Uh oh!

typescript-bot commented Jul 14, 2020

Uh oh!

reubenrybnik commented Jul 14, 2020

Uh oh!

Uh oh!

typescript-bot commented Jul 14, 2020

Uh oh!

jgonggrijp left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

typescript-bot commented Jul 15, 2020

Uh oh!

reubenrybnik commented Jul 15, 2020 •

edited

Loading

Uh oh!

jgonggrijp left a comment

Uh oh!

reubenrybnik commented Jul 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

reubenrybnik commented Jul 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

typescript-bot commented Jul 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Reviews

Status

Uh oh!

typescript-bot commented Jul 14, 2020

Uh oh!

reubenrybnik commented Jul 14, 2020

Uh oh!

Uh oh!

typescript-bot commented Jul 14, 2020

Uh oh!

jgonggrijp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

typescript-bot commented Jul 15, 2020

Uh oh!

reubenrybnik commented Jul 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jgonggrijp left a comment

Choose a reason for hiding this comment

Uh oh!

reubenrybnik commented Jul 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

reubenrybnik commented Jul 14, 2020 •

edited

Loading

typescript-bot commented Jul 14, 2020 •

edited

Loading

reubenrybnik commented Jul 15, 2020 •

edited

Loading