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

619-Migrate signature:create command #628

Merged
merged 3 commits into from
Oct 4, 2018
Merged

Conversation

mitsuaki-u
Copy link
Contributor

What was the problem?

Commander 2.0 doesn't have a way to signature for multi-signature transaction.

How did I fix it?

Create signature:create command

How to test it?

Build, lint, test.

Review checklist

@mitsuaki-u mitsuaki-u self-assigned this Oct 2, 2018
@mitsuaki-u mitsuaki-u requested a review from shuse2 October 2, 2018 10:17
@mitsuaki-u mitsuaki-u changed the title 🌱 Migrate signature:create command 619- Migrate signature:create command Oct 2, 2018
@mitsuaki-u mitsuaki-u changed the title 619- Migrate signature:create command 619-Migrate signature:create command Oct 2, 2018
shuse2
shuse2 previously requested changes Oct 2, 2018
`;

CreateCommand.examples = [
'signature:create \'{"type":0,"amount":"10","recipientId":"100L","senderPublicKey":"abcd1234","timestamp":59353522,"asset":{},"id":"abcd1234","signature":"abcd1234"}\'',
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer executable example

.stub(
elements.default.transaction,
'createSignatureObject',
transactionStub,
Copy link
Contributor

Choose a reason for hiding this comment

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

why not put the sandbox.stub().returns(defaultSignatureObject) as return value here?

@shuse2
Copy link
Contributor

shuse2 commented Oct 3, 2018

One thing I forgot to mention, we we add a new feature, let's update the README at the same time.
readme can be generated using npx oclif-dev readme --multi and

  • delete the reference to code (since it reference to dist)
  • Fix the link on the top of the page

@shuse2 shuse2 merged commit 35a128e into 2.0.0 Oct 4, 2018
@shuse2 shuse2 deleted the 619-signature_create_command branch October 4, 2018 09:37
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.

2 participants