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.
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 craneis the only command with aPersistentPreRunEfunction, other than root.PersistentPreRunEis always called for child commands, unless it's overwritten. This meanszarf tools craneand 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
https://github.com/zarf-dev/zarf/blob/3645ba22e053af8a226c380b1757794706f2e587/src/cmd/tools/crane.go#L41
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.