Skip to content
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.

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

fix: compatibility with newer ckb-cli #71

Merged
merged 1 commit into from
Oct 25, 2022

Conversation

blckngm
Copy link
Collaborator

@blckngm blckngm commented Sep 22, 2022

  • Remove --wait-for-sync arg when calling ckb-cli
  • Remove fields that no longer exists in LiveCellInfoVec

@@ -114,7 +113,6 @@ impl Collector {
.arg("--url")
.arg(&self.api_uri)
.arg("wallet")
.arg("--wait-for-sync")
Copy link
Contributor

@TheWaWaR TheWaWaR Sep 22, 2022

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Didn't notice this because I have run ckb-cli interactively before and there are saved urls.

This argument will need to be added to capsule deploy too and passed to ckb-cli?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, just like the argument --url above.

@TheWaWaR
Copy link
Contributor

Is this PR ready for merge?

@blckngm blckngm marked this pull request as ready for review September 25, 2022 08:33
@blckngm
Copy link
Collaborator Author

blckngm commented Sep 25, 2022

Is this PR ready for merge?

I think so. Please review.

@TheWaWaR
Copy link
Contributor

Please rebase and sign all your commits.

* Remove --wait-for-sync arg when calling ckb-cli

* Remove fields that no longer exists in LiveCellInfoVec

* Add --ckb-indexer-url arg to deploy command

  It could also be specified with environment variable CKB_INDEXER_URL.

  And the --api arg can be specified with the API_URL environment variable
  now.

  (These environment variable names are consistent with ckb-cli.)
@blckngm blckngm force-pushed the ckb-cli-compatibility branch from 366cdca to 4aa6742 Compare September 28, 2022 14:28
@blckngm
Copy link
Collaborator Author

blckngm commented Sep 28, 2022

Please rebase and sign all your commits.

Done.

@TheWaWaR TheWaWaR merged commit 6e3c56a into nervosnetwork:develop Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants