Skip to content

Conversation

@SimonSchick
Copy link
Contributor

Please fill in this template.

Select one of these and delete the others:

If changing an existing definition:

@typescript-bot typescript-bot added Critical package Author is Owner The author of this PR is a listed owner of the package. labels Jun 3, 2021
@typescript-bot
Copy link
Contributor

typescript-bot commented Jun 3, 2021

@SimonSchick Thank you for submitting this PR!

This is a live comment which I will keep updated.

1 package in this PR

Code Reviews

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

You can test the changes of this PR in the Playground.

Status

  • ✅ No merge conflicts
  • ✅ Continuous integration tests have passed
  • ✅ Most recent commit is approved by a DT maintainer

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": 53566,
  "author": "SimonSchick",
  "headCommitOid": "ecc153b407b16506efb9530483aeb07b43c916e8",
  "lastPushDate": "2021-06-24T17:21:15.000Z",
  "lastActivityDate": "2021-07-01T18:57:42.000Z",
  "mergeOfferDate": "2021-07-01T16:37:07.000Z",
  "mergeRequestDate": "2021-07-01T18:57:42.000Z",
  "mergeRequestUser": "SimonSchick",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "node",
      "kind": "edit",
      "files": [
        {
          "path": "types/node/buffer.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/child_process.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/crypto.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/fs/promises.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/http.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/net.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/test/buffer.ts",
          "kind": "test"
        },
        {
          "path": "types/node/test/child_process.ts",
          "kind": "test"
        },
        {
          "path": "types/node/test/crypto.ts",
          "kind": "test"
        },
        {
          "path": "types/node/test/fs.ts",
          "kind": "test"
        },
        {
          "path": "types/node/test/http.ts",
          "kind": "test"
        },
        {
          "path": "types/node/test/net.ts",
          "kind": "test"
        },
        {
          "path": "types/node/test/stream.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "Microsoft",
        "DefinitelyTyped",
        "jkomyno",
        "alvis",
        "r3nya",
        "btoueg",
        "smac89",
        "touffy",
        "DeividasBakanas",
        "eyqs",
        "Hannes-Magnusson-CK",
        "KSXGitHub",
        "hoo29",
        "kjin",
        "ajafff",
        "islishude",
        "mwiktorczyk",
        "mohsen1",
        "n-e",
        "galkin",
        "parambirs",
        "eps1lon",
        "SimonSchick",
        "ThomasdenH",
        "WilcoBakker",
        "wwwy3y3",
        "samuela",
        "kuehlein",
        "bhongy",
        "chyzwar",
        "trivikr",
        "nguymin4",
        "yoursunny",
        "qwelias",
        "ExE-Boss",
        "Ryan-Willpower",
        "peterblazejewicz",
        "addaleax",
        "JasonHK",
        "victorperin",
        "ZYSzys"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "orta",
      "date": "2021-07-01T16:36:32.000Z",
      "isMaintainer": true
    },
    {
      "type": "approved",
      "reviewer": "Mesteery",
      "date": "2021-06-24T18:06:48.000Z",
      "isMaintainer": false
    },
    {
      "type": "approved",
      "reviewer": "panva",
      "date": "2021-06-24T17:48:47.000Z",
      "isMaintainer": false
    },
    {
      "type": "stale",
      "reviewer": "dnalborczyk",
      "date": "2021-06-04T22:47:54.000Z",
      "abbrOid": "68985fc"
    }
  ],
  "mainBotCommentID": 853504139,
  "ciResult": "pass"
}

@typescript-bot
Copy link
Contributor

@typescript-bot typescript-bot added the The CI failed When GH Actions fails label Jun 3, 2021
@typescript-bot
Copy link
Contributor

@SimonSchick The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

@typescript-bot typescript-bot added The CI failed When GH Actions fails and removed The CI failed When GH Actions fails labels Jun 3, 2021
@typescript-bot
Copy link
Contributor

@SimonSchick The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

@typescript-bot typescript-bot removed the The CI failed When GH Actions fails label Jun 3, 2021
@SimonSchick SimonSchick mentioned this pull request Jun 3, 2021
17 tasks
@typescript-bot typescript-bot added the The CI failed When GH Actions fails label Jun 3, 2021
@typescript-bot
Copy link
Contributor

@SimonSchick The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

@dnalborczyk
Copy link
Contributor

just saw this comment in the v15.12 PR: #52776 (comment)

@typescript-bot typescript-bot added The CI failed When GH Actions fails and removed The CI failed When GH Actions fails labels Jun 3, 2021
@typescript-bot
Copy link
Contributor

@SimonSchick The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

@typescript-bot typescript-bot added The CI failed When GH Actions fails and removed The CI failed When GH Actions fails labels Jun 4, 2021
@typescript-bot
Copy link
Contributor

@SimonSchick The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

@typescript-bot typescript-bot removed the The CI failed When GH Actions fails label Jun 4, 2021
@typescript-bot
Copy link
Contributor

@Mesteery 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?

@SimonSchick
Copy link
Contributor Author

@peterblazejewicz can I get approval on this so I can move forward w/ v16?

@SimonSchick SimonSchick mentioned this pull request Jun 7, 2021
8 tasks
@SimonSchick
Copy link
Contributor Author

Can we move this before it gets conflicts or otherwise becomes outdated?

@typescript-bot typescript-bot added the Unreviewed No one showed up to review this PR, so it'll be reviewed by a DT maintainer. label Jun 15, 2021
@typescript-bot
Copy link
Contributor

@typescript-bot typescript-bot added the The CI failed When GH Actions fails label Jun 16, 2021
@typescript-bot
Copy link
Contributor

@SimonSchick The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

@typescript-bot typescript-bot removed the The CI failed When GH Actions fails label Jun 16, 2021
@typescript-bot
Copy link
Contributor

@dnalborczyk, @Mesteery 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?

@SimonSchick
Copy link
Contributor Author

@peterblazejewicz can I get signoff on this please <3

Copy link
Contributor

@panva panva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimonSchick

crypto:
  add keyObject.export() 'jwk' format option (Filip Skokan) #37081

and

crypto:
  support JWK objects in create*Key (Filip Skokan) #37254

from https://github.com/nodejs/node/releases/tag/v15.9.0 and https://github.com/nodejs/node/releases/tag/v15.12.0 failed to make it to your past PRs, despite being present in #52652 that you've asked to take over.

Can you add them to this one?

@typescript-bot typescript-bot added the Revision needed This PR needs code changes before it can be merged. label Jun 22, 2021
@typescript-bot
Copy link
Contributor

@SimonSchick One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits. Thank you!

@SimonSchick
Copy link
Contributor Author

Strange, something must've gone wrong in my cherry pick since most of the commits contents are in, just not these calls, I will look into it.

@typescript-bot typescript-bot removed the Revision needed This PR needs code changes before it can be merged. label Jun 24, 2021
@SimonSchick
Copy link
Contributor Author

@panva done, squashed into v15.14 commit, feel free to verify.
@peterblazejewicz can we please move forward?

@typescript-bot
Copy link
Contributor

@panva, @dnalborczyk, @Mesteery 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?

@typescript-bot typescript-bot added the Other Approved This PR was reviewed and signed-off by a community member. label Jun 24, 2021
@typescript-bot
Copy link
Contributor

@dnalborczyk 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?

@typescript-bot typescript-bot added Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner labels Jul 1, 2021
@SimonSchick
Copy link
Contributor Author

Ready to merge

@typescript-bot typescript-bot merged commit c8f0288 into DefinitelyTyped:master Jul 1, 2021
@SimonSchick SimonSchick deleted the node/v15.14 branch July 1, 2021 18:58
@typescript-bot
Copy link
Contributor

I just published @types/node@15.14.0 to npm.

@Rich-MSR
Copy link

Rich-MSR commented Jul 9, 2021

@SimonSchick
This commit breaks ExecSyncOptions. Specifically, the windowsHide property is no longer available in ExecSyncOptions.

For example:

childProcess.execSync("git status", { encoding: "utf8", windowsHide: true })

Results in:

No overload matches this call. Overload 1 of 4, '(command: string, options?: ExecSyncOptionsWithStringEncoding | undefined): string', gave the following error. Argument of type '{ encoding: "utf8"; windowsHide: boolean; }' is not assignable to parameter of type 'ExecSyncOptionsWithStringEncoding'. Object literal may only specify known properties, but 'windowsHide' does not exist in type 'ExecSyncOptionsWithStringEncoding'.

Cause:

Was: ExecSyncOptions extends CommonOptions → contains windowsHide
Now: ExecSyncOptions extends CommonExecOptions extends ProcessEnvOptions → Does not contain windowsHide

@SimonSchick SimonSchick mentioned this pull request Jul 9, 2021
8 tasks
@Rich-MSR
Copy link

Rich-MSR commented Jul 9, 2021

@SimonSchick
I've verified your #54400 fix in both @types/node@15.14.2 and @types/node@16.3.1. Thanks for your help 🙏.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Author is Owner The author of this PR is a listed owner of the package. Critical package Maintainer Approved Other Approved This PR was reviewed and signed-off by a community member. Self Merge This PR can now be self-merged by the PR author or an owner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants