Skip to content

Improve Typescript definitions #224

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

Merged
merged 4 commits into from
Mar 10, 2017
Merged

Improve Typescript definitions #224

merged 4 commits into from
Mar 10, 2017

Conversation

alex3165
Copy link
Contributor

@alex3165 alex3165 commented Mar 9, 2017

Add resultFunc, recomputations and resetRecomputations to the Typescript definition file.

This is just an adaptation of #167 from @lukevenn against the new branch 3.0.0

@codecov-io
Copy link

codecov-io commented Mar 9, 2017

Codecov Report

Merging #224 into 3.0.0 will not change coverage.
The diff coverage is n/a.

@@         Coverage Diff          @@
##           3.0.0   #224   +/-   ##
====================================
  Coverage    100%   100%           
====================================
  Files          1      1           
  Lines         15     15           
====================================
  Hits          15     15

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 801317f...809993d. Read the comment docs.

@alex3165
Copy link
Contributor Author

alex3165 commented Mar 9, 2017

Hey @threehams and @aikoven, what do you think about it ?

@coveralls
Copy link

coveralls commented Mar 9, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 2bc9a4c on typings-improvement into 801317f on 3.0.0.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 2bc9a4c on typings-improvement into 801317f on 3.0.0.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 2bc9a4c on typings-improvement into 801317f on 3.0.0.

Copy link

@lukevenn lukevenn left a comment

Choose a reason for hiding this comment

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

This looks good to me compared to my PR @alex3165 mentions. 👍

@aikoven
Copy link
Contributor

aikoven commented Mar 9, 2017

What about ParametricSelector? Is there a reason it wasn't updated?

@alex3165
Copy link
Contributor Author

alex3165 commented Mar 9, 2017

Also added a namespace to close this issue #149

@aikoven
Copy link
Contributor

aikoven commented Mar 9, 2017

Also added a namespace to close this issue #149

A more modern way of doing this is export as namespace syntax.

@coveralls
Copy link

coveralls commented Mar 9, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 6b9f951 on typings-improvement into 801317f on 3.0.0.

@coveralls
Copy link

coveralls commented Mar 9, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 42c11ff on typings-improvement into 801317f on 3.0.0.

@alex3165
Copy link
Contributor Author

alex3165 commented Mar 9, 2017

@aikoven Thanks for your feedbacks I didn't know namespace as ... I have added it and added resultFunc ... to ParametricSelector as well 👍

@aikoven
Copy link
Contributor

aikoven commented Mar 9, 2017

Thanks! It looks good now 👍

@coveralls
Copy link

coveralls commented Mar 9, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 809993d on typings-improvement into 801317f on 3.0.0.

@alex3165
Copy link
Contributor Author

alex3165 commented Mar 9, 2017

@ellbee Would it be possible to have your approval ?

@ellbee
Copy link
Collaborator

ellbee commented Mar 9, 2017

@alex3165 Nope, I have no idea about Typescript! If @aikoven is happy I assume it is good.

@alex3165
Copy link
Contributor Author

alex3165 commented Mar 9, 2017

@aikoven Should I merge this PR ?

@aikoven
Copy link
Contributor

aikoven commented Mar 9, 2017

I think so.

I would also add test for non-module usage, that uses the namespace you've added. Though I'm not sure if this is important. If you want, add another test file without imports, that uses <reference/> to link typings file.

@alex3165 alex3165 merged commit 2abc7d0 into 3.0.0 Mar 10, 2017
@timdorr timdorr deleted the typings-improvement branch May 14, 2018 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants