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

CLI: refactoring: remove Options from sc_service::Configuration's fields #5271

Merged
merged 258 commits into from
Apr 7, 2020

Conversation

cecton
Copy link
Contributor

@cecton cecton commented Mar 17, 2020

Major changes

  1. Removes VersionInfo and replace by a trait SubstrateCli that needs to be implemented on the top level structopt of the binary
  2. Add a proc_macro spec_factory to put on a spec factory/load spec function to help the impl of SubstrateCli (will automatically fill version, executable name, etc... for you)
  3. Remove Option on sc_service::Configuration's fields where it doesn't make any sense.
  4. Add a trait CliConfiguration that needs to be implemented on structopts that are considered as commands (subcommands and runcmd). This these impl are all provided by sc_cli by default but if the user add a subcommand, they will need to implement it.
  5. Add a proc_macro substrate_cli_params to help impl the CliConfiguration
  6. Replaced impl_version by full version and dropped Configuration.full_version()

Minor changes

  1. Merge Configuration.node and NetworkConfiguration.node_name because they are the same thing
  2. Removed config_path in NetworkConfiguration because it's not used and it's redundant with net_config_path
  3. Re-export NoExtension and TracingReceiver in sc_service
  4. Added a default impl for WasmExecutionMethod to Intepreted
  5. Renamed "role" to "roles" because you can be an authority and a full node
  6. ... (more details will follow)

Help needed

Please check and propose fixes for all the new documentation.

Closes #4776

polkadot companion: paritytech/polkadot#935

@cecton cecton added the A3-in_progress Pull request is in progress. No review needed at this stage. label Mar 17, 2020
@cecton cecton self-assigned this Mar 17, 2020
@parity-cla-bot
Copy link

It looks like @cecton signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

cecton added 4 commits March 17, 2020 10:58
Forked at: 2afecf8
Parent branch: origin/master
Forked at: 2afecf8
Parent branch: origin/master
cecton and others added 19 commits April 6, 2020 15:50
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
@cecton cecton force-pushed the cecton-the-revenge-of-the-cli branch from 4fa9e4a to 993b86c Compare April 6, 2020 17:36
@bkchr bkchr merged commit b0efaa2 into master Apr 7, 2020
@bkchr bkchr deleted the cecton-the-revenge-of-the-cli branch April 7, 2020 09:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Service's Configuration has optional fields that shouldn't be optional
10 participants