Skip to content

[dotenv-flow] Remove superfluous /// <reference types="node" />#46734

Merged
andrewbranch merged 1 commit intoDefinitelyTyped:masterfrom
jablko:reference/node/dotenv-flow
Aug 14, 2020
Merged

[dotenv-flow] Remove superfluous /// <reference types="node" />#46734
andrewbranch merged 1 commit intoDefinitelyTyped:masterfrom
jablko:reference/node/dotenv-flow

Conversation

@jablko
Copy link
Copy Markdown
Contributor

@jablko jablko commented Aug 13, 2020

@jablko jablko force-pushed the reference/node/dotenv-flow branch from 8fdc183 to ca4565e Compare August 13, 2020 17:52
@typescript-bot
Copy link
Copy Markdown
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?

master #46734 diff
Batch compilation
Memory usage (MiB) 62.7 36.4 -42.0%
Type count 8898 2138 -76%
Assignability cache size 1613 101 -94%
Language service
Samples taken 36 36 0%
Identifiers in tests 36 36 0%
getCompletionsAtPosition
    Mean duration (ms) 379.5 93.1 -75.5% 🌟
    Mean CV 12.3% 27.9%
    Worst duration (ms) 473.8 118.5 -75.0% 🌟
    Worst identifier encoding parsed
getQuickInfoAtPosition
    Mean duration (ms) 369.7 92.0 -75.1% 🌟
    Mean CV 11.0% 27.0% +146.5%
    Worst duration (ms) 445.6 114.5 -74.3% 🌟
    Worst identifier purge_dotenv encoding

Wow, it looks like all the big movers moved in the right direction! Way to go! 🌟 I won’t post performance data again unless it gets worse.

@typescript-bot typescript-bot added the Perf: Better typescript-bot determined that this PR improves compilation performance. label Aug 13, 2020
@jablko jablko marked this pull request as ready for review August 13, 2020 18:01
@typescript-bot typescript-bot added the Untested Change This PR does not touch tests label Aug 13, 2020
@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Aug 13, 2020

@jablko Thank you for submitting this PR!

This is a live comment which I will keep updated.

This PR doesn't modify any tests, so it's hard to know what's being fixed, and your changes might regress in the future. Have you considered adding tests to cover the change you're making? Including tests allows this PR to be merged by yourself and the owners of this module. This can potentially save days of time for you.

Code Reviews

This PR can be merged once it's reviewed by a DT maintainer.

Status

  • ✅ No merge conflicts
  • ✅ Continuous integration tests have passed
  • ✅ Only a DT maintainer can approve 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": 46734,
  "author": "jablko",
  "owners": [
    "vincentlanglet",
    "kerimdzhanov"
  ],
  "dangerLevel": "ScopedAndUntested",
  "headCommitAbbrOid": "ca4565e",
  "headCommitOid": "ca4565e80451e18e1fcca6e3332527aae70c9522",
  "mergeIsRequested": false,
  "stalenessInDays": 0,
  "lastPushDate": "2020-08-13T17:52:24.000Z",
  "reopenedDate": "2020-08-13T18:01:24.000Z",
  "lastCommentDate": "2020-08-13T17:52:24.000Z",
  "maintainerBlessed": false,
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/46734/files",
  "hasMergeConflict": false,
  "authorIsOwner": false,
  "isFirstContribution": false,
  "popularityLevel": "Well-liked by everyone",
  "anyPackageIsNew": false,
  "packages": [
    "dotenv-flow"
  ],
  "files": [
    {
      "path": "types/dotenv-flow/index.d.ts",
      "kind": "definition",
      "package": "dotenv-flow"
    }
  ],
  "hasDismissedReview": false,
  "ciResult": "pass",
  "lastReviewDate": "2020-08-14T09:38:41.000Z",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 6,
  "isChangesRequested": false
}

@typescript-bot
Copy link
Copy Markdown
Contributor

🔔 @VincentLanglet @kerimdzhanov — 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.

Copy link
Copy Markdown
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

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

🌻

@typescript-bot typescript-bot added Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner labels Aug 13, 2020
Copy link
Copy Markdown
Contributor

@kerimdzhanov kerimdzhanov left a comment

Choose a reason for hiding this comment

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

Ready to merge

@typescript-bot typescript-bot added the Owner Approved A listed owner of this package signed off on the pull request. label Aug 14, 2020
@andrewbranch andrewbranch merged commit fc17c7f into DefinitelyTyped:master Aug 14, 2020
@typescript-bot
Copy link
Copy Markdown
Contributor

I just published @types/dotenv-flow@3.0.1 to npm.

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

Labels

Maintainer Approved Owner Approved A listed owner of this package signed off on the pull request. Perf: Better typescript-bot determined that this PR improves compilation performance. Self Merge This PR can now be self-merged by the PR author or an owner Untested Change This PR does not touch tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants