Skip to content
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.

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

prefix experimental react-dom apis with unstable #44598

Closed
wants to merge 1 commit into from
Closed

prefix experimental react-dom apis with unstable #44598

wants to merge 1 commit into from

Conversation

raghav0622
Copy link

Prefix experimental react-dom apis with unstable with refrence to https://github.com/DefinitelyTyped/DefinitelyTyped

@typescript-bot
Copy link
Contributor

typescript-bot commented May 9, 2020

@raghav0622 Thank you for submitting this PR! I see this is your first time submitting to DefinitelyTyped 👋 - keep an eye on this comment as I'll be updating it with information as things progress.

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

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": 44598,
  "author": "raghav0622",
  "owners": [
    "MartynasZilinskas",
    "theruther4d",
    "Jessidhia"
  ],
  "dangerLevel": "ScopedAndUntested",
  "headCommitAbbrOid": "9e42638",
  "headCommitOid": "9e4263857af822957c79a0c5eb4790ab1c4a9c1f",
  "mergeIsRequested": false,
  "stalenessInDays": 0,
  "lastCommitDate": "2020-05-09T15:06:58.000Z",
  "lastCommentDate": "2020-05-09T19:31:42.000Z",
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/44598/files",
  "hasMergeConflict": false,
  "authorIsOwner": false,
  "isFirstContribution": true,
  "popularityLevel": "Critical",
  "anyPackageIsNew": false,
  "packages": [
    "react-dom"
  ],
  "files": [
    {
      "filePath": "types/react-dom/experimental.d.ts",
      "kind": "definition",
      "package": "react-dom"
    }
  ],
  "hasDismissedReview": false,
  "travisResult": "fail",
  "travisUrl": "https://travis-ci.org/github/DefinitelyTyped/DefinitelyTyped/builds/685073497?utm_source=github_status&utm_medium=notification",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 0,
  "isChangesRequested": false
}

@typescript-bot
Copy link
Contributor

🔔 @MartynasZilinskas @theruther4d @Jessidhia - 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.

@typescript-bot typescript-bot moved this from Other to Waiting for Code Reviews in New Pull Request Status Board May 9, 2020
@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Author Action in New Pull Request Status Board May 9, 2020
@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?

Comparison details 📊
master #44598 diff
Batch compilation
Memory usage (MiB) 108.1 110.5 +2.3%
Type count 24825 24821 0%
Assignability cache size 69318 69314 0%
Language service
Samples taken 434 434 0%
Identifiers in tests 434 434 0%
getCompletionsAtPosition
    Mean duration (ms) 544.0 553.9 +1.8%
    Mean CV 7.8% 8.0%
    Worst duration (ms) 719.1 732.0 +1.8%
    Worst identifier component rootElement
getQuickInfoAtPosition
    Mean duration (ms) 541.1 547.9 +1.3%
    Mean CV 7.7% 7.5% -2.5%
    Worst duration (ms) 717.6 761.2 +6.1%
    Worst identifier createElement createElement

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 9, 2020
@leonardodino
Copy link
Contributor

duplicate of #44564

@Jessidhia
Copy link
Member

#44564 (which is a superset of this PR) was merged

@Jessidhia Jessidhia closed this May 11, 2020
@typescript-bot typescript-bot removed this from Needs Author Action in New Pull Request Status Board May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Critical package Perf: Same typescript-bot determined that this PR will not significantly impact compilation performance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants