Skip to content

Task/g195 rxjs compatibility reopen #211

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 18 commits into from
Jul 26, 2019
Merged

Task/g195 rxjs compatibility reopen #211

merged 18 commits into from
Jul 26, 2019

Conversation

@rg911
Copy link
Contributor

@rg911 rg911 commented Jul 23, 2019

Issue #195

Problem

After updated rxjs to the latest version (run npm update) it throw compile error on all http repositories. Latest version of Rxjs and typescript only support strong typed parameters.

Fixes

  • Update the swagger codegen typescript-node template with correct resolve & reject codes.
  • Update Http repos with strong typed parameters matching the generated swagger codes.
  • Added errorHandling to parse useful error details. (generated swagger code returns the full http response).
  • Tested on current & latest npm packages (after npm update)

rg911 and others added 7 commits July 16, 2019 15:22
@coveralls
Copy link

coveralls commented Jul 23, 2019 •
edited
Loading

Pull Request Test Coverage Report for Build 592

  • 20 of 207 (9.66%) changed or added relevant lines in 17 files are covered.
  • 6 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.3%) to 60.455%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/model/account/Account.ts 0 1 0.0%
src/infrastructure/Http.ts 2 4 50.0%
src/infrastructure/api/networkRoutesApi.ts 0 2 0.0%
src/infrastructure/NetworkHttp.ts 4 8 50.0%
src/infrastructure/api/chainRoutesApi.ts 0 4 0.0%
src/infrastructure/api/diagnosticRoutesApi.ts 0 4 0.0%
src/infrastructure/api/nodeRoutesApi.ts 0 4 0.0%
src/infrastructure/api/mosaicRoutesApi.ts 0 6 0.0%
src/infrastructure/api/namespaceRoutesApi.ts 0 8 0.0%
src/infrastructure/api/blockRoutesApi.ts 0 12 0.0%
Files with Coverage Reduction New Missed Lines %
src/infrastructure/NamespaceHttp.ts 2 16.07%
src/infrastructure/TransactionHttp.ts 4 25.93%
Totals Coverage Status
Change from base Build 584: -0.3%
Covered Lines: 6363
Relevant Lines: 9095
💛 - Coveralls

rg911 added 11 commits July 25, 2019 12:47
@rg911
Added symbol#204
b02cfea 
- Fixed signTransactionGivenSignatures bug
- Added e2e test for signTransactionGivenSignatures
- Improved unit test for signTransactionGivenSignatures
- Fixed typo
@rg911
Added signSchema to signWithPayload
a5a6b25
@rg911
Exposed signTransactionGivenSignatures method
5874599
@rg911
Merge branch 'task/g204_signTransactionGivenSignatures' of github.com…
6f3a288 
…:rg911/nem2-sdk-typescript-javascript into task/g204_signTransactionGivenSignatures
@rg911
Merge pull request symbol#206 from rg911/task/g204_signTransactionGiv…
716c4db 
…enSignatures

Task/g204 sign transaction given signatures
@rg911
Merge branch 'master' into patch-2
b3aaaff
@rg911
Merge pull request symbol#210 from jontey/patch-2
501842e 
Fixed version
@rg911
Added symbol#195 fixed compatibility issue on new rxjs version
3279d93
@rg911
Fixed error handling
46141f9
@rg911
Fixed transactionHttp unit test
3f67948
@rg911
Merge branch 'task/g195_rxjs_compatibility_reopen' of github.com:rg91…
f990421 
…1/nem2-sdk-typescript-javascript into task/g195_rxjs_compatibility_reopen
evias
evias approved these changes Jul 26, 2019
View reviewed changes
Copy link
Contributor

@evias evias left a comment

Choose a reason for hiding this comment

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

tested with client app

@rg911 rg911 merged commit 3256618 into symbol:master Jul 26, 2019
@rg911 rg911 deleted the task/g195_rxjs_compatibility_reopen branch October 8, 2019 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@evias evias evias approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rg911 @coveralls @evias @jontey