Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

Non-upgradeable deploys, round 2 #1376

Merged
merged 87 commits into from
Jan 21, 2020

Conversation

frangio
Copy link
Contributor

@frangio frangio commented Jan 7, 2020

Supercedes #1240. Fixes #802.

This is quite a large pull request that adds a new deploy command that can be used for deploying both upgradeable and "regular" contracts.

The new command was built following a different architecture than all the current commands, as an experiment to see how we can refactor the entire commands layer to have better separation of concerns and better maintainability. The idea would be to refactor the rest of the codebase if we agree that this is an improvement.

In the current architecture there are command and script layers. The command layer provides a command's specification and action, and the action handles interactivity very explicitly by making use of the prompts module in the action flow. The script layer is very thin and almost all it does is error handling, and printing to stdout, then delegating to a controller.

The proposed architecture removes the scripts layer, and separates the command layer more explicitly into command specification and action. Interactivity is no longer a concern of the action, instead the command specification provides details for the behavior of each positional argument and option, and when the command is registered in the commander program these details are used to run interactive prompts prior to running the action.

The code for registering a command using this new architecture can be found in the register-command.ts file, including also the code for running the interactive prompts based on a command spec. There are no tests for this, but I think I should add a few before merging this.

The two parts of the new command are found in commands/deploy/spec.ts and commands/deploy/action.ts. This separation will allow for optimizing the loading time of the CLI because the dependencies of the action will not be imported unless the command is actually called. There is also commands/deploy/index.ts that provides the glue between the proposed architecture and the current architecture.

One of the things that I think should be improved is the naming around prompt, Question, etc., in the args and opts specification. I kind of picked these names quickly and kept them but they're not good.

@frangio frangio merged commit e789b22 into OpenZeppelin:master Jan 21, 2020
@frangio frangio deleted the feature/regular-deploys-2 branch January 21, 2020 19:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status:to-review Awaiting review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creating non-upgradeable contracts
3 participants