@JasonHK Thank you for submitting this PR!

Because this is a new definition, a DefinitelyTyped maintainer will be reviewing this PR in the next few days once the Travis CI build passes.

In the meantime, if the build fails or a merge conflict occurs, I'll let you know. Have a nice day!

@jkomyno, @a-tarasyuk, @alvis, @r3nya, @btoueg, @BrunoScheufler, @smac89, @tellnes, @Touffy, @DeividasBakanas, @eyqs, @Flarna, @Hannes-Magnusson-CK, @KSXGitHub, @hoo29, @kjin, @ajafff, @islishude, @mwiktorczyk, @mohsen1, @n-e, @octo-sniffle, @galkin, @parambirs, @eps1lon, @SimonSchick, @ThomasdenH, @WilcoBakker, @wwwy3y3, @samuela, @kuehlein, @j-oliveras, @bhongy, @chyzwar, @trivikr, @nguymin4, @yoursunny, @qwelias, @ExE-Boss, @Ryan-Willpower, @peterblazejewicz, @JasonHK

I appreciate if any of you owners could take a look at this PR.

@jkomyno, @a-tarasyuk, @alvis, @r3nya, @btoueg, @BrunoScheufler, @smac89, @tellnes, @Touffy, @DeividasBakanas, @eyqs, @Flarna, @Hannes-Magnusson-CK, @KSXGitHub, @hoo29, @kjin, @ajafff, @islishude, @mwiktorczyk, @mohsen1, @n-e, @octo-sniffle, @galkin, @parambirs, @eps1lon, @SimonSchick, @ThomasdenH, @WilcoBakker, @wwwy3y3, @samuela, @kuehlein, @j-oliveras, @bhongy, @chyzwar, @trivikr, @nguymin4, @yoursunny, @qwelias, @ExE-Boss, @Ryan-Willpower, @peterblazejewicz, @JasonHK

I appreciate if any of you owners could take a look at this PR.

Actually I'm not the code owner (yet).

After 5 days, no one has reviewed the PR 😞. A maintainer will be reviewing the PR in the next few days and will either merge it or request revisions. Thank you for your patience!

Not my level yet, sadly! As the topic is recurring, I think someone probably would be better suited (sorry mate(s) for pointing with finger):
/cc @ExE-Boss
previously subject of: #43308

Not my level yet, sadly! As the topic is recurring, I think someone probably would be better suited (sorry mate(s) for pointing with finger):
/cc @ExE-Boss
previously subject of: #43308

@peterblazejewicz That PR was made by me as well, that PR was closed automatically since I don't have time to resolve the conflict before.

@JasonHK Unfortunately, this pull request currently has a merge conflict 😥. Please update your PR branch to be up-to-date with respect to master. Have a nice day!

Not my level yet, sadly! As the topic is recurring, I think someone probably would be better suited (sorry mate(s) for pointing with finger):
/cc @ExE-Boss
previously subject of: #43308

@peterblazejewicz How can I improve this PR to the level of quality you expect?

In my opinion, this is the minimum changes required to bring globalThis support to global.

I know some properties in NodeJS.Global that will be duplicated after this PR, but I think we could fix them later, since these won't cause any errors in the mean time.

And I also have a package that were benefited from this PR, since global as unknown as NodeJS.Global | typeof globalThis shouldn't be the way to solve this problem.

@JasonHK 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.

{
  "type": "info",
  "pr_number": 43821,
  "author": "JasonHK",
  "owners": [],
  "dangerLevel": "MultiplePackagesEdited",
  "headCommitAbbrOid": "57aac40",
  "headCommitOid": "57aac40341ce048b3f7706ca18bd010fb160aff4",
  "mergeIsRequested": false,
  "stalenessInDays": 17,
  "lastCommitDate": "2020-04-12T07:46:49.000Z",
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/43821/files",
  "hasMergeConflict": true,
  "authorIsOwner": false,
  "isFirstContribution": false,
  "popularityLevel": "Critical",
  "anyPackageIsNew": false,
  "packages": [
    "node",
    "window-or-global"
  ],
  "files": [
    {
      "filePath": "types/node/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/index.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/package.json",
      "kind": "package-meta",
      "package": "node"
    },
    {
      "filePath": "types/node/ts3.2/base.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/ts3.2/index.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/ts3.4/base.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/ts3.4/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/ts3.4/index.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/ts3.4/node-tests.ts",
      "kind": "test",
      "package": "node"
    },
    {
      "filePath": "types/node/ts3.4/tsconfig.json",
      "kind": "package-meta",
      "package": "node"
    },
    {
      "filePath": "types/node/ts3.4/tslint.json",
      "kind": "package-meta",
      "package": "node"
    },
    {
      "filePath": "types/node/ts3.5/base.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/ts3.5/index.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/index.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/package.json",
      "kind": "package-meta",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/ts3.2/base.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/ts3.2/index.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/ts3.4/base.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/ts3.4/globals.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/ts3.4/index.d.ts",
      "kind": "definition",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/ts3.4/node-tests.ts",
      "kind": "test",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/ts3.4/tsconfig.json",
      "kind": "package-meta",
      "package": "node"
    },
    {
      "filePath": "types/node/v12/ts3.4/tslint.json",
      "kind": "package-meta",
      "package": "node"
    },
    {
      "filePath": "types/window-or-global/index.d.ts",
      "kind": "definition",
      "package": "window-or-global"
    },
    {
      "filePath": "types/window-or-global/package.json",
      "kind": "package-meta",
      "package": "window-or-global"
    },
    {
      "filePath": "types/window-or-global/ts3.4/index.d.ts",
      "kind": "definition",
      "package": "window-or-global"
    },
    {
      "filePath": "types/window-or-global/ts3.4/tsconfig.json",
      "kind": "package-meta",
      "package": "window-or-global"
    },
    {
      "filePath": "types/window-or-global/ts3.4/tslint.json",
      "kind": "package-meta",
      "package": "window-or-global"
    },
    {
      "filePath": "types/window-or-global/ts3.4/window-or-global-tests.ts",
      "kind": "test",
      "package": "window-or-global"
    }
  ],
  "otherApprovalCount": 0,
  "ownerApprovalCount": 0,
  "maintainerApprovalCount": 0,
  "hasDismissedReview": false,
  "travisResult": "pass",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 0,
  "isChangesRequested": false
}

@JasonHK Unfortunately, this pull request currently has a merge conflict 😥. Please update your PR branch to be up-to-date with respect to master. Have a nice day!

@JasonHK To keep things tidy, we have to close PRs that aren't mergeable but don't have activity from their author. No worries, though - please open a new PR if you'd like to continue with this change. Thank you!

@JasonHK
sorry if you felt that way, your PR is 100% correct 👍 . My intent was to remark the one-of-the latest dicussion on same topic, due to nature of the package.

Please rebase your branch from the current master and resubmit, then redo the PR. There is a lot of topics going around NodeJS at the ment on the repo, with a lot of concerns related to changes, that could somehow impact existing user base. So just be prepared for discussion. I understand the change, due to short time of being involved directly into the Node dt package, I would probably not have an opinion on the change itself :)
Thanks!

@JasonHK
sorry if you felt that way, your PR is 100% correct +1 . My intent was to remark the one-of-the latest dicussion on same topic, due to nature of the package.

Please rebase your branch from the current master and resubmit, then redo the PR. There is a lot of topics going around NodeJS at the ment on the repo, with a lot of concerns related to changes, that could somehow impact existing user base. So just be prepared for discussion. I understand the change, due to short time of being involved directly into the Node dt package, I would probably not have an opinion on the change itself :)
Thanks!

@peterblazejewicz I'm already prepared for discussion, but it seems that they don't want to discuss with me...

The only "discussion" is within the first PR (#43308), one of the owners, he asked me is it worth to add globalThis support to the definition, I explained to him, then I never heard anything back.

Same as the reviewer, he said he need motivation for it, I told him why this is important, then I also never heard anything back.

After that, none of them comment to my PRs (except you), including the current PR (#44700).