[@types/underscore] Collection and Array Tests - Iteratee Cleanup#46634
Merged
typescript-bot merged 6 commits intoDefinitelyTyped:masterfrom Aug 11, 2020
Merged
Conversation
…have been updated to return collection types where appropriate.
… sortedIndex include undefined as a possibility for when it is called to transform the value argument.
…t but that I don't have a better solution for.
…what it used to be to reduce errors from functions that haven't been updated to use a correct wrapped value yet (e.g. array proxy functions).
Contributor
|
@reubenrybnik Thank you for submitting this PR! This is a live comment which I will keep updated. Code ReviewsBecause you edited one package and updated the tests (👏), I can help you merge this PR once someone else signs off on it. Status
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": 46634,
"author": "reubenrybnik",
"owners": [
"borisyankov",
"jbaldwin",
"ccurrens",
"confususs",
"jgonggrijp",
"ffflorian",
"regevbr",
"peterblazejewicz",
"reubenrybnik"
],
"dangerLevel": "ScopedAndTested",
"headCommitAbbrOid": "3be612c",
"headCommitOid": "3be612c0e680cbb07e35b022fd292db45507f54f",
"mergeIsRequested": true,
"stalenessInDays": 0,
"lastPushDate": "2020-08-09T18:08:27.000Z",
"lastCommentDate": "2020-08-11T20:38:29.000Z",
"maintainerBlessed": false,
"reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/46634/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-08-11T19:58:22.000Z",
"reviewersWithStaleReviews": [],
"approvalFlags": 2,
"isChangesRequested": false
} |
Contributor
|
🔔 @borisyankov @jbaldwin @ccurrens @confususs @jgonggrijp @ffflorian @regevbr @peterblazejewicz — please review this PR in the next few days. Be sure to explicitly select |
reubenrybnik
commented
Aug 9, 2020
reubenrybnik
commented
Aug 9, 2020
Contributor
|
👋 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 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. |
…as also what was historically assumed there by iterators if nothing else.
Contributor
|
Updated numbers for you here from 5a9a5c7. Comparison details 📊
Looks like there were a couple significant differences—take a look at worst-case duration for getting quick info at a position to make sure everything looks ok. |
jgonggrijp
approved these changes
Aug 10, 2020
Contributor
jgonggrijp
left a comment
There was a problem hiding this comment.
I have some comments, but overall it looks OK to me.
jgonggrijp
approved these changes
Aug 11, 2020
Contributor
Author
|
Ready to merge |
chivesrs
pushed a commit
to chivesrs/DefinitelyTyped
that referenced
this pull request
Sep 2, 2020
…rray Tests - Iteratee Cleanup by @reubenrybnik * Replacing _ChainIteratee with Iteratee now that wrapped return types have been updated to return collection types where appropriate. * Making it so the CollectionIterator key and collection parameters for sortedIndex include undefined as a possibility for when it is called to transform the value argument. * Updating iteratee type comments in tests to better match Underscore nomenclature. * Updating a test to reflect a compromise that I'm not super happy about but that I don't have a better solution for. * Changing the default wrapped value for _Chain to T[] to better match what it used to be to reduce errors from functions that haven't been updated to use a correct wrapped value yet (e.g. array proxy functions). * Also updating the default V for Underscore <T, V> to T[] since that was also what was historically assumed there by iterators if nothing else.
danielrearden
pushed a commit
to danielrearden/DefinitelyTyped
that referenced
this pull request
Sep 22, 2020
…rray Tests - Iteratee Cleanup by @reubenrybnik * Replacing _ChainIteratee with Iteratee now that wrapped return types have been updated to return collection types where appropriate. * Making it so the CollectionIterator key and collection parameters for sortedIndex include undefined as a possibility for when it is called to transform the value argument. * Updating iteratee type comments in tests to better match Underscore nomenclature. * Updating a test to reflect a compromise that I'm not super happy about but that I don't have a better solution for. * Changing the default wrapped value for _Chain to T[] to better match what it used to be to reduce errors from functions that haven't been updated to use a correct wrapped value yet (e.g. array proxy functions). * Also updating the default V for Underscore <T, V> to T[] since that was also what was historically assumed there by iterators if nothing else.
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
npm test.)npm run lint package-name(ortscif notslint.jsonis present).This is the first of three planned cleanup PRs for #45201 and includes the following changes:
TtoT[]to better match what it used to be to reduce errors from functions that haven't been updated to use a correct wrapped value yet (e.g. array proxy functions). This also allows the simplification of many_Chaintype references.