-
Notifications
You must be signed in to change notification settings - Fork 69
Extract code from batches package into smaller sub-packages #511
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
Conversation
|
Pushed another commit that creates the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks for doing this! Can't wait to see what this develops into.. 😬
internal/batches/step_changes.go
Outdated
| @@ -0,0 +1 @@ | |||
| package batches | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this file required for anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uhhh, no. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels sensible to me. Let's get it in and see how it feels as we use it!
This is a follow-up to #511 and #512. It cleans up the code that's shared between `batch apply` and `previe` by trying to make the boundaries between the "TUI + execution"-glue-code, the `service` pkg, the `executor` pkg and the helpers clearer. A lot of this is subjective, of course (what does "clearer" even mean, man), but I think less duplication is Good and less cross-talk between a trio of components too.
This is a follow-up to #511 and #512. It cleans up the code that's shared between `batch apply` and `previe` by trying to make the boundaries between the "TUI + execution"-glue-code, the `service` pkg, the `executor` pkg and the helpers clearer. A lot of this is subjective, of course (what does "clearer" even mean, man), but I think less duplication is Good and less cross-talk between a trio of components too.
* Clean up shared code between `batch apply` and `preview` This is a follow-up to #511 and #512. It cleans up the code that's shared between `batch apply` and `previe` by trying to make the boundaries between the "TUI + execution"-glue-code, the `service` pkg, the `executor` pkg and the helpers clearer. A lot of this is subjective, of course (what does "clearer" even mean, man), but I think less duplication is Good and less cross-talk between a trio of components too. * Remove commented out code * Fix comment
* Create executor, git, service packages * Rename Opts in executor * Rename ServiceOpts * Move LogManager to log package * workspace pkg, unexport types and rename WorkspaceCreator to Creator * Remove unused file
* Clean up shared code between `batch apply` and `preview` This is a follow-up to #511 and #512. It cleans up the code that's shared between `batch apply` and `previe` by trying to make the boundaries between the "TUI + execution"-glue-code, the `service` pkg, the `executor` pkg and the helpers clearer. A lot of this is subjective, of course (what does "clearer" even mean, man), but I think less duplication is Good and less cross-talk between a trio of components too. * Remove commented out code * Fix comment
This is the first step towards cleaning up this code base a bit.
It extracts code into smaller sub-packages:
executormockgitlogserviceworkspaceAnd it renames some of the types so they make more sense in this new structure:
batches.ExecutorOptstoexecutor.Optsfor example.This doesn't magically fix problems, but it makes it easier to see where dependencies and strong coupling exists.
And it's only the first step: there's still a few TODOs that I added. Next I want to clear up the interplay between
serviceandexecutorand themainpackage.I opened this PR to make reviewing easier: it doesn't change behaviour but only renames and moves things