Skip to content

Use chia cli parser#116

Merged
altendky merged 26 commits into
ericaltendorf:developmentfrom
altendky:use_chia_cli_parser
May 2, 2021
Merged

Use chia cli parser#116
altendky merged 26 commits into
ericaltendorf:developmentfrom
altendky:use_chia_cli_parser

Conversation

@altendky

@altendky altendky commented Apr 16, 2021

Copy link
Copy Markdown
Collaborator

Draft for:

@altendky altendky changed the base branch from main to development April 16, 2021 03:11
@ericaltendorf

Copy link
Copy Markdown
Owner

This is great!

@altendky

Copy link
Copy Markdown
Collaborator Author

Note that this is still hand waving in terms of the subcommand parsing. There could be arguments interleaved for parent subcommands in the command line without making it so that it isn't a plot command (present code would ignore such a process). There may be more but at at least venv/bin/chia --root-path my_path_to_chia_stuff plots create should be valid. But, I think it's fair to leave that for 'maybe later'. Also, the choice of -h and --help is hard coded since command.get_help_option_names(ctx=context) only returns --help (comment left in the relevant code). I didn't dig hard into the details of that.

@altendky altendky marked this pull request as ready for review April 18, 2021 14:17

@chelseadole chelseadole left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me :)

@altendky

Copy link
Copy Markdown
Collaborator Author

This got reworked to copy in the Chia command parsing code rather than having the dependency on the chia-blockchain package. In various cases people are installing chia without the Python package such as .dmg on macOS. This approach also allows us to store multiple versions and in the future we could attempt to identify the version of the process and pick the proper parsing code for it. Anyways... maybe this is ready for real review and merging so we can move on from all these complaints...

@altendky altendky marked this pull request as ready for review April 30, 2021 02:21
@altendky altendky requested a review from ericaltendorf April 30, 2021 02:22

@ericaltendorf ericaltendorf left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It seems unfortunate that we need to copy chia code over, but if that's what seems like the best option, let's go for it. Should it be documented somewhere what's going on? I.e., which code came from chia and what the process should be for updating it / keeping it in sync?

Comment thread src/plotman/chia.py
@altendky

altendky commented May 2, 2021

Copy link
Copy Markdown
Collaborator Author

Alrighty, there's some documentation. Let me know if it is clear.

@ericaltendorf

Copy link
Copy Markdown
Owner

Makes sense, thanks!

@altendky altendky merged commit bccf012 into ericaltendorf:development May 2, 2021
@altendky altendky deleted the use_chia_cli_parser branch May 2, 2021 17:51
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.

3 participants