Update local deployment guide. Improve CLI commands. #485

sept-en · 2021-02-08T15:59:18Z

Fixes #467

mfornet

@sept-en can you rebase on top of master, such that only commits relevant to this PR are taking into account.

sept-en · 2021-02-15T12:27:13Z

@mfornet now only relevant commits are applied on top of master.

README.md

mfornet · 2021-02-15T12:42:03Z

cli/index.js

+  [
+    'near-master-account',
+    'near-erc20-account',
+    'near-network-id',
+    'near-node-url',
+  ]


Missing near-receiver-account

Offtopic of this PR, but this is a reason why we need more consistent way to handle CLI, using a framework such as yargs.

mfornet · 2021-02-15T12:44:53Z

testing/adapter/index.js

+// View
+async function getBridgeOnNearBalance ({
+  nearReceiverAccount,
+  nearMasterAccount: nearMasterAccountId,


Why using different name here?

Suggested change

nearMasterAccount: nearMasterAccountId,

nearMasterAccount,

mfornet · 2021-02-15T12:45:15Z

testing/adapter/index.js

+    )
+
+    const nearTokenContract = new nearAPI.Contract(
+      nearMasterAccount,


This is undefined at this point

mfornet · 2021-02-15T12:47:15Z

cli/index.js

+      '--near-receiver-account <near_receiver_account>',
+      'The account on NEAR blockchain that received the minted token.'
+    )
+    .description('Gets balance on Near for Rainbow-Bridge Eth on Near Prover.'),


Why Near Prover?

Wrong info related to the old account that was used everywhere.

cli/index.js

README.md

mfornet · 2021-02-15T12:51:32Z

README.md

@@ -69,7 +69,7 @@ To learn the commands that you can use with the rainbow bridge run
 Alternatively, clone this repo, `yarn install`, then you can see what commands you can use with:

 ```
-    ./index.js --help
+    cli/index.js --help


Throughout the README the command rainbow is being used. Either change it to use cli/index.js everywhere, or use rainbow here.

The rainbow --help command is mentioned a few lines above.
Therefore, I believe that it's good to have cli/index.js here, so if someone decides to test using local code, it's clear.

On a second thought, for the sake of consistency, can you remove first two instructions:

(1) npm i -g rainbow-bridge-cli (2) rainbow --help

Since we are not releasing new versions. If a developer tries that it will just fail. So let's stick with the cli/index.js interface for now on all the steps.

README.md

mfornet

LGTM. Please address both comments.

mfornet · 2021-02-16T22:21:07Z

README.md

@@ -69,7 +69,7 @@ To learn the commands that you can use with the rainbow bridge run
 Alternatively, clone this repo, `yarn install`, then you can see what commands you can use with:

 ```
-    ./index.js --help
+    cli/index.js --help


On a second thought, for the sake of consistency, can you remove first two instructions:

(1) npm i -g rainbow-bridge-cli (2) rainbow --help

Since we are not releasing new versions. If a developer tries that it will just fail. So let's stick with the cli/index.js interface for now on all the steps.

mfornet · 2021-02-16T22:33:44Z

testing/adapter/index.js

+  nearNodeUrl,
+}) {
+  try {
+    // @ts-ignore


remove

Suggested change

// @ts-ignore

Ok, got it. I agree in this case it's better to stick to cli/index.js approach.

* CLI: make transfer functions callable directly from CLI (without "TESTING" prefix). * CLI: improve `getErc20Balance` method. * CLI: add `getBridgeOnNearBalance` method. * Update README. * Minor improvements.

* CLI: make aux methods (transfer and view-balance functionality) to be called through the TESTING subcommand again. * CLI: `getBridgeOnNearBalance` - don't provide private key to the view method. * Update README.

Co-authored-by: Marcelo Fornet <mfornet94@gmail.com>

mfornet · 2021-02-18T16:17:14Z

@sept-en address formatting issues, right now we are using standardjs. Fix with:

yarn
yarn standard --fix

* (#467) Minor CLI improvements. Update README. * CLI: make transfer functions callable directly from CLI (without "TESTING" prefix). * CLI: improve `getErc20Balance` method. * CLI: add `getBridgeOnNearBalance` method. * Update README. * Minor improvements. * (#467) Update README. CLI commands minor changes. * CLI: make aux methods (transfer and view-balance functionality) to be called through the TESTING subcommand again. * CLI: `getBridgeOnNearBalance` - don't provide private key to the view method. * Update README. * (#467) Update README. * Update cli/index.js Co-authored-by: Marcelo Fornet <mfornet94@gmail.com> * Update README Co-authored-by: Marcelo Fornet <mfornet94@gmail.com> * Update README. Minor improvements and fixes. * Update README: use `cli/index.js` as a rainbow cmd. * Remove redundant code. * Lint fixes. Co-authored-by: Marcelo Fornet <mfornet94@gmail.com>

mfornet force-pushed the beta branch from c3da5d5 to 9738242 Compare February 8, 2021 17:10

sept-en requested review from abacabadabacaba and mfornet as code owners February 8, 2021 17:10

mfornet changed the base branch from beta to master February 8, 2021 17:14

mfornet suggested changes Feb 12, 2021

View reviewed changes

sept-en force-pushed the issue-467 branch from a20485c to 6c22fe3 Compare February 15, 2021 12:25

sept-en requested a review from mfornet February 15, 2021 12:27

mfornet suggested changes Feb 15, 2021

View reviewed changes

mfornet approved these changes Feb 16, 2021

View reviewed changes

sept-en and others added 8 commits February 17, 2021 22:26

(Near-One#467) Minor CLI improvements. Update README.

0fb88bf

* CLI: make transfer functions callable directly from CLI (without "TESTING" prefix). * CLI: improve `getErc20Balance` method. * CLI: add `getBridgeOnNearBalance` method. * Update README. * Minor improvements.

(Near-One#467) Update README. CLI commands minor changes.

3004dc2

* CLI: make aux methods (transfer and view-balance functionality) to be called through the TESTING subcommand again. * CLI: `getBridgeOnNearBalance` - don't provide private key to the view method. * Update README.

(Near-One#467) Update README.

39ac1fa

Update cli/index.js

86d0339

Co-authored-by: Marcelo Fornet <mfornet94@gmail.com>

Update README

ed2e3cf

Co-authored-by: Marcelo Fornet <mfornet94@gmail.com>

Update README. Minor improvements and fixes.

4b9250b

Update README: use cli/index.js as a rainbow cmd.

8166685

Remove redundant code.

16f0786

sept-en force-pushed the issue-467 branch from c4d30e6 to 16f0786 Compare February 17, 2021 20:33

Lint fixes.

fcc9800

mfornet enabled auto-merge (squash) February 18, 2021 16:33

mfornet merged commit 4ee7472 into Near-One:master Feb 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update local deployment guide. Improve CLI commands. #485

Update local deployment guide. Improve CLI commands. #485

sept-en commented Feb 8, 2021 •

edited by chadoh

Loading

mfornet left a comment

sept-en commented Feb 15, 2021

mfornet Feb 15, 2021

mfornet Feb 15, 2021

mfornet Feb 15, 2021

mfornet Feb 15, 2021

sept-en Feb 15, 2021

mfornet Feb 15, 2021

sept-en Feb 15, 2021

mfornet Feb 16, 2021

mfornet left a comment

mfornet Feb 16, 2021

mfornet Feb 16, 2021

sept-en Feb 17, 2021

mfornet commented Feb 18, 2021

Update local deployment guide. Improve CLI commands. #485

Update local deployment guide. Improve CLI commands. #485

Conversation

sept-en commented Feb 8, 2021 • edited by chadoh Loading

mfornet left a comment

Choose a reason for hiding this comment

sept-en commented Feb 15, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mfornet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mfornet commented Feb 18, 2021

sept-en commented Feb 8, 2021 •

edited by chadoh

Loading