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

chore: reorganize config into separate module #1479

Merged
merged 3 commits into from
Jul 25, 2024

Conversation

willemneal
Copy link
Member

@willemneal willemneal commented Jul 23, 2024

Initially the config was a subcommand and so it was mixed in with the commands. This separates it out so that we can begin the process of pulling it out into it's own crate.

This should become it's own crate to allow devs to use accounts and aliases when compiling contract.

Initially the config was a subcommand and so it was mixed in with the commands. This separates it out so that we can begin the process of pulling it out into it's own crate.
@janewang janewang added the cli Related to Soroban CLI label Jul 23, 2024
@willemneal willemneal self-assigned this Jul 23, 2024
Copy link
Member

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

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

Feels right to move logic like the shared args out of the network commands module into a shared place due to the common nature and the args not being scoped to the network command.

The refactor looks good to me in that context 👏🏻 .

This separates it out so that we can begin the process of pulling it out into it's own crate.

That being said, the refactor groups cli code together, but doesn't separate out config parsing/writing logic from tightly coupled cli logic, so I'm not sure this change sufficiently prepares a module for extraction to another crate.

This should become it's own crate to allow devs to use accounts and aliases when compiling contract.

Could you share more about this workflow? I wouldn't expect compiling contracts to embed aliases or account data into contracts, and even if the cli needed to do that the cli can already access them.

I am probably missing a critical part of how this will serve folks, but moving config into a separate crate seems very low impact / priority at the moment. What am I missing?

@leighmcculloch leighmcculloch merged commit d0c077b into stellar:main Jul 25, 2024
23 checks passed
@chadoh chadoh deleted the chore/reorg_config branch September 23, 2024 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to Soroban CLI
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants