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

feat: Validator extension #67

Closed
wants to merge 24 commits into from
Closed

feat: Validator extension #67

wants to merge 24 commits into from

Conversation

volovyks
Copy link
Contributor

@volovyks volovyks commented Sep 23, 2021

Validator extension works at this point, but I can not say that I'm happy with the result. Here is the path that I suggest we should follow in the future:

  • use near-jsonrpc-client-rs instead of nearcore
  • use macros from this PR (after the merge)
  • create near-cli-builder crate inside of this repo, use it in the core project and validators extension (now validators extension is a separate project)

In general, 5000+ lines of code for an extension with 2 commands is too much. Even after the creation of macros mentioned above, we will not reach the point when users will be able to build new extensions without pain. I hope that a bunch of macros and libraries mentioned in this PR will solve these issues.

@volovyks volovyks self-assigned this Sep 23, 2021
@volovyks volovyks marked this pull request as draft September 23, 2021 14:53
@volovyks volovyks linked an issue Sep 23, 2021 that may be closed by this pull request
@frol
Copy link
Collaborator

frol commented Sep 24, 2021

From the first glance, the extension is totally fine from the implementation point of view, though I would spend a few moments thinking through the structure (your README features near validator list while the implementation expects near validator validators; both variants are a bit odd for different reasons)

In general, 5000+ lines of code for an extension with 2 commands is too much.

I totally agree. I feel sorry that you had to do it manually. Nobody should struggle over and over again here!

Even after the creation of macros mentioned above, we will not reach the point when users will be able to build new extensions without pain. I hope that a bunch of macros and libraries mentioned in this PR will solve these issues.

I also have high hopes for that. I hope we can get to the point where we can have so little code that there is nothing to remove from it. Would you simply use clap, your CLI implementation would fit in a few hundred lines. We should definitely challenge ourselves to reach that state with the available automation (derive macros, generics, etc).

@frol
Copy link
Collaborator

frol commented Dec 28, 2021

@FroVolod Please, refactor validators extension to interactive-clap and create a separate branch to compare

…on errors on response due to missing `runtime_config`)
@frol
Copy link
Collaborator

frol commented Jan 4, 2022

@volovyk-s Hey, check out this refactored version using the interactive-clap macro: https://github.com/near/near-cli-rs/tree/validator-extension-interactive-clap/extensions/validator/src

@Buckram123
Copy link
Contributor

Buckram123 commented Mar 10, 2022

@frol Build fails on my machine

error: failed to download `near-account-id v0.11.0`

Caused by:
  unable to get packages from source

Caused by:
  failed to parse manifest at `/Users/anymacstore/.cargo/registry/src/github.com-1ecc6299db9ec823/near-account-id-0.11.0/Cargo.toml`

Caused by:
  feature `edition2021` is required

  this Cargo does not support nightly features, but if you
  switch to nightly channel you can add
  `cargo-features = ["edition2021"]` to enable this feature

shouldn't rust-toolchain be updated to 1.56.0+?

@frol
Copy link
Collaborator

frol commented Mar 15, 2022

@Buckram123 This PR should be closed in favor of #93

@frol frol closed this Mar 15, 2022
@frol frol deleted the validator-extension branch February 11, 2023 15:27
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.

Validators extension
3 participants