CommandRunner: apply separation of concern #310
Comments
|
@damithc @eugenepeh your opinion on this? |
|
Sounds good to me. |
|
part of #442 |
eugenepeh
added a commit
that referenced
this issue
Dec 22, 2018
All the git commands are embedded in the CommandRunner class. This violates the Single Responsibility Principle as CommandRunner should only be responsible for the execution, and not the type of commands. As a step towards deprecating the use of CommandRunner, let's export the commands to respective classes and remove the direct dependency on CommandRunner.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
CommandRunner currently handles executing all of the 'system'
command that we have specified.
This violates the Single Responsibility Principle.
The current design is also inflexible, as whenever we wish to add
a new command (or an existing command with different parameters),
we would need to hardcode a new method for it.
We can consider using the Command + Builder pattern, like JGit GitCommands
to split the responsiblities of each 'common' command to their own classes.
This will allow for commands to be reused with new parameters (without
affecting existing usages), and also
easier unit testing as we can control which parameter to add (instead of
having to set up the entire
RepoConfiguration).The text was updated successfully, but these errors were encountered: