Skip to content
This repository has been archived by the owner on Apr 6, 2020. It is now read-only.

Update Travis to run on Node v8 and v10; Add tests for inherited FakeTransaction functions #138

Merged
merged 6 commits into from
Apr 16, 2019

Conversation

chikeichan
Copy link
Contributor

Fixes #114

Opening this one on top of #131 - I will change merge target to master when #131 is merged.

@coveralls
Copy link

coveralls commented Mar 5, 2019

Coverage Status

Coverage remained the same at 98.936% when pulling 37cc36f on 114 into 8e425ec on master.

@holgerd77
Copy link
Member

This has a linting error, also Node 6 is not supporting the spread operator. That said we have just decided to drop Node 6 support and you can use this PR to update CI to run towards Node 8, 10 and 11 (if you do can you then add a note to your initial PR description so that this won't get forgotten on release notes).

@holgerd77
Copy link
Member

One procedural note: can you use the PR-marked labels for your pull requests - primarily the WIP and Ready-for-review (only once tests passes) labels? Then it get's easier to judge if a PR is ready. You can also request specific reviewers if you like (not necessarily me).

@holgerd77
Copy link
Member

That said: thanks for the various PRs! 👍 😄

@chikeichan
Copy link
Contributor Author

One procedural note: can you use the PR-marked labels for your pull requests - primarily the WIP and Ready-for-review (only once tests passes) labels? Then it get's easier to judge if a PR is ready. You can also request specific reviewers if you like (not necessarily me).

Thanks for the housekeeping notes @holgerd77 ! I will address the comments on various PRs over the next 3-5 days

@holgerd77
Copy link
Member

👍

@chikeichan chikeichan changed the title Add tests for inherited FakeTransaction functions Update Travis to run on Node v8, 10, 11; Add tests for inherited FakeTransaction functions Mar 11, 2019
@chikeichan chikeichan changed the title Update Travis to run on Node v8, 10, 11; Add tests for inherited FakeTransaction functions Update Travis to run on Node v8 and v10; Add tests for inherited FakeTransaction functions Mar 11, 2019
@danjm danjm force-pushed the update-official-transaction-tests branch 2 times, most recently from d7c98ea to 4d75731 Compare March 31, 2019 01:30
@holgerd77
Copy link
Member

Hi @chikeichan can you bring this up-to-date?

@chikeichan chikeichan changed the base branch from update-official-transaction-tests to master April 16, 2019 00:09
@alcuadrado
Copy link
Member

This looks good to me. The only method not directly tested is getSenderPublicKey, but it gets tested though getSenderAddress.

About the travis part. Should it include v11? It is the Current version now, but that'll change in a few weeks, and its EOL is in two months.

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

Review by @alcuadrado, will merge, thanks @chikeichan!

@holgerd77 holgerd77 merged commit 80eb355 into master Apr 16, 2019
@holgerd77 holgerd77 deleted the 114 branch April 16, 2019 20:37
@alcuadrado alcuadrado mentioned this pull request Apr 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tests for inherited FakeTransaction functions
5 participants