Skip to content

change function value type to generic function interface #1163

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Jan 16, 2019
Merged

change function value type to generic function interface #1163

merged 13 commits into from
Jan 16, 2019

Conversation

tkow
Copy link
Contributor

@tkow tkow commented Jan 10, 2019

  • change type arrow function to generic function interface to enable to preserve it generic parametric function type.

see i18next/react-i18next#665 ,too.

@coveralls
Copy link

coveralls commented Jan 10, 2019 •
edited
Loading

Coverage Status

Coverage remained the same at 88.413% when pulling 813985a on tkow:feature/generic-translate-function into c1358a6 on i18next:master.

@tkow tkow mentioned this pull request Jan 10, 2019
Merged
@tkow tkow mentioned this pull request Jan 10, 2019
Closed
@jamuhl
Copy link
Member

jamuhl commented Jan 10, 2019

@rosskevin...might ask again for help for this one?

@rosskevin rosskevin self-assigned this Jan 10, 2019
@rosskevin
Copy link
Collaborator

I'm going to hold this PR until we have baseline tests in place.

@tkow
Copy link
Contributor Author

tkow commented Jan 11, 2019 •
edited
Loading

@jamuhl I copy ts tests and configure linter referring to DefinitlyTyped. About this PR, it may satisfy enough no breaking compatibility of typing.

@tkow tkow force-pushed the feature/generic-translate-function branch from 4c452b6 to 361cb92 Compare January 11, 2019 05:42
@tkow
Copy link
Contributor Author

tkow commented Jan 11, 2019 •
edited
Loading

I'm sorry, I have a mistake to contaminate WithT interface commit. Then I rebased it, it doesn't include in this PR now. Please check it.

@jamuhl jamuhl mentioned this pull request Jan 11, 2019
Merged
@rosskevin rosskevin removed the pr hold label Jan 11, 2019
@rosskevin
Copy link
Collaborator

rosskevin commented Jan 11, 2019 •
edited
Loading

@tkow this PR is now breaking - and looking at it now that we have tests running on master. The tests are breaking even though it doesn't indicate so. Please pull my updates and see https://travis-ci.org/i18next/i18next/builds/478478184 and look at fixing. You can run npm run test:typescript to test. Also there is a npm run prettier:typescript so we can avoid style changes in diffs.

rosskevin
rosskevin requested changes Jan 11, 2019
View reviewed changes
Copy link
Collaborator

@rosskevin rosskevin left a comment

Choose a reason for hiding this comment

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

broken ci even though github doesn't indicate it https://travis-ci.org/i18next/i18next/builds/478478184

@tkow
Copy link
Contributor Author

tkow commented Jan 12, 2019 •
edited
Loading

@rosskevin
This test had included some not compatibility types, so I fixed it.
https://travis-ci.org/i18next/i18next/builds/478656682

(Addition)
I confirmed ,even given that added to current master , the old tests failed, so old dts tests were broken from before ... so, now it should fix correct 😃

@tkow tkow mentioned this pull request Jan 12, 2019
Closed
@rosskevin rosskevin removed the pr hold label Jan 16, 2019
@rosskevin rosskevin merged commit 0c60262 into i18next:master Jan 16, 2019
@panjiesw panjiesw mentioned this pull request Jan 19, 2019
Merged
@rosskevin rosskevin mentioned this pull request Jan 23, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rosskevin rosskevin rosskevin requested changes

Assignees

@rosskevin rosskevin

Labels
typescript
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tkow @coveralls @jamuhl @rosskevin