Skip to content

Add alternative labeled tuple printback for parameter capture lists expected in tests #44897

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 19, 2020
Merged

Add alternative labeled tuple printback for parameter capture lists expected in tests #44897

merged 2 commits into from
May 19, 2020

Conversation

weswigham
Copy link
Contributor

This should be all the changes microsoft/TypeScript#38234 needs to be clean on DT.

@weswigham weswigham requested a review from borisyankov as a code owner May 19, 2020 20:57
@typescript-bot
Copy link
Contributor

typescript-bot commented May 19, 2020 •
edited
Loading

@weswigham Thank you for submitting this PR!

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

Status

  • ✅ No merge conflicts
  • ✅ Continuous integration tests have passed
  • ❌ Only a DT maintainer can merge changes without tests

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Diagnostic Information: What the bot saw about this PR 
{
  "type": "info",
  "now": "-",
  "pr_number": 44897,
  "author": "weswigham",
  "owners": [
    "ExE-Boss",
    "ljharb",
    "borisyankov",
    "theodorejb",
    "davidparsson",
    "gmoothart",
    "lukas-zech-software",
    "Engineer2B",
    "cyungmann",
    "Roaders",
    "devoto13",
    "fdim",
    "kolodny",
    "stephenfarrar",
    "zvirja",
    "djungowski",
    "chivesrs",
    "NoHomey",
    "jwbay",
    "asvetliakov",
    "alexjoverm",
    "epicallan",
    "ikatyang",
    "wsmd",
    "JamieMason",
    "douglasduteil",
    "ahnpnl",
    "joshuakgoldberg",
    "UselessPickles",
    "r3nya",
    "hotell",
    "sebald",
    "andys8",
    "antoinebrault",
    "favna",
    "gstamac",
    "quassnoi",
    "Belco90",
    "tonyhallett",
    "ycmjason",
    "devanshj",
    "pawfa",
    "regevbr",
    "maranomynet",
    "spacejack",
    "isiahmeadows",
    "mrbigdog2u",
    "rationull",
    "lumaxis",
    "nicojs",
    "43081j",
    "gjednaszewski",
    "johnjesse",
    "alecf",
    "SimonSchick",
    "bergundy"
  ],
  "dangerLevel": "MultiplePackagesEdited",
  "headCommitAbbrOid": "230bf6d",
  "headCommitOid": "230bf6d980ece9741b1951596726e8741556cb71",
  "mergeIsRequested": false,
  "stalenessInDays": 0,
  "lastCommitDate": "2020-05-19T21:08:50.000Z",
  "lastCommentDate": "2020-05-19T21:01:42.000Z",
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/44897/files",
  "hasMergeConflict": false,
  "authorIsOwner": false,
  "isFirstContribution": false,
  "popularityLevel": "Critical",
  "anyPackageIsNew": false,
  "packages": [
    "es-abstract",
    "jasmine",
    "jest",
    "ospec",
    "sinon"
  ],
  "files": [
    {
      "filePath": "types/es-abstract/test/helpers/callBind.test.ts",
      "kind": "test",
      "package": "es-abstract"
    },
    {
      "filePath": "types/jasmine/ts3.1/jasmine-tests.ts",
      "kind": "test",
      "package": "jasmine"
    },
    {
      "filePath": "types/jest/jest-tests.ts",
      "kind": "test",
      "package": "jest"
    },
    {
      "filePath": "types/ospec/ospec-tests.ts",
      "kind": "test",
      "package": "ospec"
    },
    {
      "filePath": "types/sinon/ts3.1/sinon-tests.ts",
      "kind": "test",
      "package": "sinon"
    }
  ],
  "hasDismissedReview": false,
  "travisResult": "pass",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 0,
  "isChangesRequested": false
}

@typescript-bot
Copy link
Contributor

⚠️ There are too many reviewers for this PR change (56). Merging can only be handled by a DT maintainer.

People who would have been pinged ExE-Boss ljharb borisyankov theodorejb davidparsson gmoothart lukas-zech-software Engineer2B cyungmann Roaders devoto13 fdim kolodny stephenfarrar zvirja djungowski chivesrs NoHomey jwbay asvetliakov alexjoverm epicallan ikatyang wsmd JamieMason douglasduteil ahnpnl joshuakgoldberg UselessPickles r3nya hotell sebald andys8 antoinebrault favna gstamac quassnoi Belco90 tonyhallett ycmjason devanshj pawfa regevbr maranomynet spacejack isiahmeadows mrbigdog2u rationull lumaxis nicojs 43081j gjednaszewski johnjesse alecf SimonSchick bergundy

@danger-public
Copy link

danger-public commented May 19, 2020 •
edited
Loading

Inspecting the JavaScript source for this package found some properties that are not in the .d.ts files.
The check for missing properties isn't always right, so take this list as advice, not a requirement.

ospec (unpkg)

was missing the following properties:

  1. cleanStackTrace

Generated by 🚫 dangerJS against 230bf6d

@weswigham weswigham merged commit d34a1b6 into master May 19, 2020
@weswigham weswigham deleted the named-tuple-expects branch May 19, 2020 21:19
@typescript-bot
Copy link
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?

es-abstract/v*

Comparison details for es-abstract/* 📊
master #44897 diff
Batch compilation
Memory usage (MiB) 40.1 40.7 +1.5%
Type count 6291 6291 0%
Assignability cache size 3118 3118 0%
Language service
Samples taken 1522 1522 0%
Identifiers in tests 1522 1522 0%
getCompletionsAtPosition
    Mean duration (ms) 114.1 114.1 +0.1%
    Mean CV 11.5% 11.4%
    Worst duration (ms) 203.9 243.4 +19.3%
    Worst identifier RemovedKeys RemovedKeys
getQuickInfoAtPosition
    Mean duration (ms) 115.0 115.3 +0.3%
    Mean CV 11.1% 11.1% +0.1%
    Worst duration (ms) 217.3 219.5 +1.0%
    Worst identifier RemovedKeysActual AllAddedKeysActual

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

jasmine/v*

Comparison details for jasmine/* 📊
master #44897 diff
Batch compilation
Memory usage (MiB) 79.7 78.1 -2.1%
Type count 13856 13856 0%
Assignability cache size 36167 36167 0%
Language service
Samples taken 2604 2604 0%
Identifiers in tests 2604 2604 0%
getCompletionsAtPosition
    Mean duration (ms) 324.5 319.3 -1.6%
    Mean CV 8.8% 8.9%
    Worst duration (ms) 438.2 447.7 +2.2%
    Worst identifier onload onload
getQuickInfoAtPosition
    Mean duration (ms) 315.8 310.6 -1.6%
    Mean CV 8.8% 8.8% +0.3%
    Worst duration (ms) 406.3 427.0 +5.1%
    Worst identifier fn1 createSpyObj

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

jest/v*

Comparison details for jest/* 📊
master #44897 diff
Batch compilation
Memory usage (MiB) 83.2 88.6 +6.4%
Type count 14713 14713 0%
Assignability cache size 35944 35944 0%
Language service
Samples taken 2557 2557 0%
Identifiers in tests 2557 2557 0%
getCompletionsAtPosition
    Mean duration (ms) 338.1 339.6 +0.4%
    Mean CV 9.2% 9.2%
    Worst duration (ms) 445.6 442.0 -0.8%
    Worst identifier toHaveBeenCalledTimes mock
getQuickInfoAtPosition
    Mean duration (ms) 334.3 335.5 +0.3%
    Mean CV 9.3% 9.1% -1.6%
    Worst duration (ms) 451.1 429.4 -4.8%
    Worst identifier only prop

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

ospec/v*

Comparison details for ospec/* 📊
master #44897 diff
Batch compilation
Memory usage (MiB) 32.5 32.6 +0.4%
Type count 2517 2517 0%
Assignability cache size 264 264 0%
Language service
Samples taken 349 349 0%
Identifiers in tests 349 349 0%
getCompletionsAtPosition
    Mean duration (ms) 66.3 66.1 -0.4%
    Mean CV 17.0% 16.8%
    Worst duration (ms) 106.6 92.8 -12.9%
    Worst identifier bool arr
getQuickInfoAtPosition
    Mean duration (ms) 64.8 65.3 +0.9%
    Mean CV 15.4% 16.0% +3.6%
    Worst duration (ms) 92.7 94.3 +1.7%
    Worst identifier notDeepEquals notThrows

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

sinon/v*

Comparison details for sinon/* 📊
master #44897 diff
Batch compilation
Memory usage (MiB) 86.3 87.3 +1.2%
Type count 12889 12889 0%
Assignability cache size 35153 35153 0%
Language service
Samples taken 1186 1186 0%
Identifiers in tests 1186 1186 0%
getCompletionsAtPosition
    Mean duration (ms) 311.5 312.6 +0.4%
    Mean CV 9.3% 9.4%
    Worst duration (ms) 392.8 385.6 -1.8%
    Worst identifier sinon sinon
getQuickInfoAtPosition
    Mean duration (ms) 304.2 304.2 0.0%
    Mean CV 9.7% 9.7% -0.2%
    Worst duration (ms) 388.9 413.7 +6.4%
    Worst identifier sb stub

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

@typescript-bot typescript-bot added the Perf: Same typescript-bot determined that this PR will not significantly impact compilation performance. label May 20, 2020
jjballano-qatium pushed a commit to jjballano-qatium/DefinitelyTyped that referenced this pull request Jun 16, 2020
@weswigham @jjballano-qatium
Add alternative labeled tuple printback for parameter capture lists e…
f953b67 
…xpected in tests (DefinitelyTyped#44897)

* Add alternative labeled tuple printback for parameter capture lists expected in tests

* Small fix
ngbrown pushed a commit to ngbrown-forks/DefinitelyTyped that referenced this pull request Jul 11, 2020
@weswigham @ngbrown
Add alternative labeled tuple printback for parameter capture lists e…
b2e6649 
…xpected in tests (DefinitelyTyped#44897)

* Add alternative labeled tuple printback for parameter capture lists expected in tests

* Small fix
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

Critical package Perf: Same typescript-bot determined that this PR will not significantly impact compilation performance.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@weswigham @typescript-bot @danger-public