You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Describe what should be investigated or refactored
We should delete the packages src/cmd/tools and src/cmd/common and move the code into src/cmd. Having src/cmd/tools separate from from the rest of src/cmd forces us to maintain src/cmd/common to share functions used by both packages. I do not think the benefit of having separate packages warrant the extra complexity in this case.
The current setup makes sharing code between packages harder. For example, zarf tools crane is the only command with a PersistentPreRunE function, other than root. PersistentPreRunE is always called for child commands, unless it's overwritten. This means zarf tools crane and it's children are the only commands that don't run the root pre-run. This makes it harder to setup a logger. This is problematic because we wrap crane commands with additional logic, and add Zarf specific commands to crane.
Ideally, we could call the root pre-run function at the start of the crane pre-run, however it would cause an import cycle if we did that since src/cmd/tools is currently already imported by src/cmd. We could instead add the root pre-run function to common, but it would be simpler to move the code together
We have a helm package under src/tools. IMO this can stay separate as it's mostly a means of implementing the helm commands we can not import as a library.
The text was updated successfully, but these errors were encountered:
Describe what should be investigated or refactored
We should delete the packages src/cmd/tools and src/cmd/common and move the code into src/cmd. Having src/cmd/tools separate from from the rest of src/cmd forces us to maintain src/cmd/common to share functions used by both packages. I do not think the benefit of having separate packages warrant the extra complexity in this case.
The current setup makes sharing code between packages harder. For example,
zarf tools crane
is the only command with aPersistentPreRunE
function, other than root.PersistentPreRunE
is always called for child commands, unless it's overwritten. This meanszarf tools crane
and it's children are the only commands that don't run the root pre-run. This makes it harder to setup a logger. This is problematic because we wrap crane commands with additional logic, and add Zarf specific commands to crane.Ideally, we could call the root pre-run function at the start of the crane pre-run, however it would cause an import cycle if we did that since src/cmd/tools is currently already imported by src/cmd. We could instead add the root pre-run function to common, but it would be simpler to move the code together
Links to any relevant code
zarf/src/cmd/tools/crane.go
Line 41 in 3645ba2
Additional context
We have a helm package under src/tools. IMO this can stay separate as it's mostly a means of implementing the helm commands we can not import as a library.
The text was updated successfully, but these errors were encountered: