Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Update angular-resource.d.ts - #4721

#4721
Closed
ilgb wants to merge 1 commit into
DefinitelyTyped:masterDefinitelyTyped/DefinitelyTyped:masterfrom
ilgb:masterilgb/DefinitelyTyped:masterCopy head branch name to clipboard
Closed

Update angular-resource.d.ts#4721
ilgb wants to merge 1 commit into
DefinitelyTyped:masterDefinitelyTyped/DefinitelyTyped:masterfrom
ilgb:masterilgb/DefinitelyTyped:masterCopy head branch name to clipboard

Conversation

@ilgb

@ilgb ilgb commented Jun 24, 2015

Copy link
Copy Markdown

Replace methods return type T by IResource in IResourceClass

@ilgb ilgb mentioned this pull request Jun 25, 2015
Closed
@vvakame

vvakame commented Jun 27, 2015

Copy link
Copy Markdown
Member

@diegovilar @miffels could you review this PR?

@miffels

miffels commented Jun 27, 2015

Copy link
Copy Markdown
Contributor

Seems correct, no idea how I missed it back then: Source

@ilgb Please add a test or two that exercises the return values of the IResourceClass interface.

Otherwise 👍

@sandcastle

sandcastle commented Dec 19, 2015

Copy link
Copy Markdown

@miffels what are you looking as far as tests? The existing test file contains calls for all of the relevant methods:

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/angularjs/angular-resource-tests.ts#L38-L84

/CC @ilgb

@miffels

miffels commented Dec 24, 2015

Copy link
Copy Markdown
Contributor

@sandcastle Oh, I did not assume the change was backwards-compatible with the tests, so I expected the tests had to be changed, to. Turns out this addition actually is covered.

:shipit: 👍

vvakame
vvakame reviewed Dec 27, 2015
View reviewed changes
Comment thread angularjs/angular-resource.d.ts

@vvakame vvakame Dec 27, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

um? is this right?
now typescript is support intersection types. microsoft/TypeScript#3622
I think we should use get(): T & IResource<T>;. isn't it?

@RyanCavanaugh

RyanCavanaugh commented Jun 30, 2016

Copy link
Copy Markdown
Member

Hello and thank you for your contribution!

Due to an excessively long queue of pull requests that have become stale, we are "declaring bankruptcy" and closing all PRs opened before May 1, 2016. If you'd still like to merge this code in, please open a new PR that has been merged and rebased with the master branch.

Going forward, we are committing to review or merge all PRs on a regular basis so this bankruptcy will not occur again. We apologize for the incovenience and hope you will continue to contribute to DefinitelyTyped in the future.

Thanks
The TypeScript and DefinitelyTyped teams

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ilgb @vvakame @miffels @sandcastle @RyanCavanaugh
Morty Proxy This is a proxified and sanitized view of the page, visit original site.