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: tmux command execution #59

Merged
merged 2 commits into from
Feb 4, 2024

Conversation

markfeinstein
Copy link
Contributor

Restructuring the tmux package to use a central struct for command execution to help with testability.

To limit the scope of the this PR it includes the use of a package level instance of the new tmux.Command struct and an init function to initialize it. Once all of the functions using the tmux cli are updated to use the Command struct directly that code can be removed in favor of a single tmux.Command instance configured in the cli.

Tested locally and ran unit tests.

Restructuring the tmux package to use a central struct for command execution to help
with testability.

To limit the scope of the this PR it includes the use of a package level instance of
the new tmux.Command struct and an init function to initialize it. Once all of the
functions using the tmux cli are updated to use the Command struct directly that
code can be removed in favor of a single tmux.Command instance configured in the cli.
Copy link
Owner

@joshmedeski joshmedeski left a comment

Choose a reason for hiding this comment

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

Just a couple of questions about the code, thanks for moving forward with better testing

tmux/tmux.go Show resolved Hide resolved
.github/workflows/ci-cd.yml Show resolved Hide resolved
@joshmedeski joshmedeski merged commit 960bd75 into joshmedeski:main Feb 4, 2024
4 checks passed
joshmedeski added a commit that referenced this pull request Feb 4, 2024
joshmedeski added a commit that referenced this pull request Feb 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants