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

Make Vimeo request() generic to optionally type results #44316

Merged
merged 3 commits into from
May 4, 2020

Conversation

mattleff
Copy link
Contributor

@mattleff mattleff commented Apr 28, 2020

Please fill in this template.

  • Use a meaningful title for the pull request. Include the name of the package modified.
  • Test the change in your own code. (Compile and run.)
  • Add or edit tests to reflect the change. (Run with npm test.)
  • Follow the advice from the readme.
  • Avoid common mistakes.
  • Run npm run lint package-name (or tsc if no tslint.json is present).

If changing an existing definition:

  • Provide a URL to documentation or source code which provides context for the suggested changes: <>
  • If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the header.
  • Include tests for your changes
  • If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }. If for reason the any rule need to be disabled, disable it for that line using // tslint:disable-next-line [ruleName] and not for whole package so that the need for disabling can be reviewed.

@typescript-bot
Copy link
Contributor

typescript-bot commented Apr 28, 2020

@mattleff Thank you for submitting this PR!

Code Reviews

There aren't any other owners of this package, so a DT maintainer will review it.

Status

  • ✅ No merge conflicts
  • ✅ Continuous integration tests have passed
  • ✅ A DT maintainer can merge changes when there are no other reviewers

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": 44316,
  "author": "mattleff",
  "owners": [
    "mattleff"
  ],
  "dangerLevel": "ScopedAndConfiguration",
  "headCommitAbbrOid": "8e49bd4",
  "headCommitOid": "8e49bd4f15ff7e2f31677a019a208b951a0ccc93",
  "mergeIsRequested": true,
  "stalenessInDays": 0,
  "lastCommitDate": "2020-05-04T16:30:10.000Z",
  "lastAuthorCommentDate": "2020-05-04T18:06:48.000Z",
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/44316/files",
  "hasMergeConflict": false,
  "authorIsOwner": true,
  "isFirstContribution": false,
  "popularityLevel": "Well-liked by everyone",
  "anyPackageIsNew": false,
  "packages": [
    "vimeo"
  ],
  "files": [
    {
      "filePath": "types/vimeo/index.d.ts",
      "kind": "definition",
      "package": "vimeo"
    },
    {
      "filePath": "types/vimeo/tsconfig.json",
      "kind": "package-meta",
      "package": "vimeo"
    },
    {
      "filePath": "types/vimeo/vimeo-tests.ts",
      "kind": "test",
      "package": "vimeo"
    }
  ],
  "hasDismissedReview": false,
  "travisResult": "pass",
  "lastReviewDate": "2020-05-04T18:05:03.000Z",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 4,
  "isChangesRequested": false
}

@typescript-bot typescript-bot added Where is Travis? Author is Owner The author of this PR is a listed owner of the package. labels Apr 28, 2020
@typescript-bot
Copy link
Contributor

@mattleff The Travis 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 moved this from Other to Needs Author Action in New Pull Request Status Board Apr 28, 2020
@typescript-bot typescript-bot moved this from Needs Author Action to Other in New Pull Request Status Board Apr 28, 2020
@typescript-bot typescript-bot moved this from Needs Author Action to Other in New Pull Request Status Board Apr 28, 2020
@typescript-bot typescript-bot moved this from Other to Needs Author Action in New Pull Request Status Board Apr 28, 2020
@typescript-bot typescript-bot moved this from Other to Needs Author Action in New Pull Request Status Board Apr 28, 2020
@typescript-bot typescript-bot moved this from Needs Author Action to Waiting for Code Reviews in New Pull Request Status Board Apr 28, 2020
@andrewbranch
Copy link
Member

ping @typescript-bot, what’s with the labels

@typescript-bot typescript-bot removed Author is Owner The author of this PR is a listed owner of the package. The Travis CI build failed labels Apr 29, 2020
@typescript-bot
Copy link
Contributor

@mattleff The Travis 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 moved this from Waiting for Code Reviews to Needs Author Action in New Pull Request Status Board Apr 29, 2020
@typescript-bot typescript-bot moved this from Needs Author Action to Waiting for Code Reviews in New Pull Request Status Board Apr 29, 2020
@typescript-bot typescript-bot added the Author is Owner The author of this PR is a listed owner of the package. label Apr 30, 2020
@typescript-bot
Copy link
Contributor

🔔 @mattleff - 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.

@mattleff
Copy link
Contributor Author

mattleff commented May 4, 2020

@andrewbranch Thanks for fixing the dt-mergebot issues. Is this PR good to go now?

@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in New Pull Request Status Board May 4, 2020
types/vimeo/index.d.ts Outdated Show resolved Hide resolved
@typescript-bot typescript-bot moved this from Needs Maintainer Review to Waiting for Code Reviews in New Pull Request Status Board May 4, 2020
@typescript-bot
Copy link
Contributor

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

Copy link
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.

Thanks!

@typescript-bot typescript-bot added Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner labels May 4, 2020
@typescript-bot typescript-bot moved this from Waiting for Code Reviews to Waiting for Author to Merge in New Pull Request Status Board May 4, 2020
@typescript-bot
Copy link
Contributor

@mattleff Everything looks good here. Great job! I am ready to merge this PR on your behalf.

If you'd like that to happen, please post a comment saying:

Ready to merge

and I'll merge this PR almost instantly. Thanks for helping out! ❤️

@mattleff
Copy link
Contributor Author

mattleff commented May 4, 2020

Ready to merge

@typescript-bot typescript-bot moved this from Waiting for Author to Merge to Recently Merged in New Pull Request Status Board May 4, 2020
@typescript-bot typescript-bot merged commit c9dae84 into DefinitelyTyped:master May 4, 2020
@mattleff mattleff deleted the vimeo-generic branch May 4, 2020 18:06
jjballano-qatium pushed a commit to jjballano-qatium/DefinitelyTyped that referenced this pull request Jun 16, 2020
…onally type results by @mattleff

* Make Vimeo request() generic to optionally type results

* Add tests for vimeo types

* Don't type request() method
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. Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants