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

Refactor Command classes to make run method the public API #1577

Merged
merged 2 commits into from
Jul 21, 2023

Conversation

paracycle
Copy link
Member

@paracycle paracycle commented Jul 21, 2023

Motivation

Finally getting around to #1218 (review) comment by @Morriar.

By making the abstract execute method be a private method, we can ensure that all commands are invoked via the run method, which allows us to wrap them in Tapioca.silence_warnings today and potentially do more in the future.

Implementation

Add Command#run method that calls Command#execute and make Command#execute a private method. Encapsulate all Tapioca.silence_warnings calls.

Tests

There should be no behaviour change.

By making the abstract `execute` method be a private method, we can
ensure that all commands are invoked via the `run` method, which allows us to wrap them in `Tapioca.silence_warnings` today and potentially do more in the future.
@paracycle paracycle added the refactor Code refactor with no behaviour change label Jul 21, 2023
@paracycle paracycle requested a review from a team as a code owner July 21, 2023 17:42
Copy link
Contributor

@egiurleo egiurleo left a comment

Choose a reason for hiding this comment

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

😍

@paracycle paracycle merged commit f84ae89 into main Jul 21, 2023
30 checks passed
@paracycle paracycle deleted the uk-refactor-commands branch July 21, 2023 21:08
@shopify-shipit shopify-shipit bot temporarily deployed to production September 13, 2023 22:55 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code refactor with no behaviour change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants