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

Enhancement: remove the CommandInstance construct #1821

Closed
waldekmastykarz opened this issue Sep 18, 2020 · 1 comment
Closed

Enhancement: remove the CommandInstance construct #1821

waldekmastykarz opened this issue Sep 18, 2020 · 1 comment

Comments

@waldekmastykarz
Copy link
Member

Related to #1737

After removing dependency on Vorpal, we'll be able to further refactor the command code and remove the CommandInstance construct. It will help us simplify code and tests and remove obsolete code.

If I understood it correctly, Vorpal requires CommandInstance to store information about the current command in its execution context. Since out setup is easier and the context doesn't span beyond the execution of the single command, we can access all the necessary information from the command itself without having to copy it to the context.

I'll pick up this refactoring, after we closed #1737.

@waldekmastykarz
Copy link
Member Author

On a related note, we can also simplify validation. Rather than returning a function or undefined, we can implement validation directly in the validate method and by default have the base Command class return true which means that validation passed. I can include this change in the same refactoring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant