Closes #413

[Xeckt]

This PR fixes #413

I know that using init() funcs are not the ideal approach in most cases and is often disregarded and recommended against in Go. But, for the Cobra CLI and the structure of the current source I thought it be necessary for specific reasons. Cobra also recommend this way.

However, I did this a little different to try to avoid the problems with init() as much as possible.

Comparatively to the original issue #413:

1. Global slice of *cobra.command for the cmd pkg
2. Each command file has an init() function that appends the command to the global slice, this is separate from the command functions themselves

This way, we kept the original structure of the command functions so that init() doesn't interfere with the testing of the commands. And, if need be, the functions can be used elsewhere.

I decided this was the better approach for init() to make it only valuable for the command slice, and not replace the functions directly with init().

I have also renamed the functions for simpler naming, rather than NewXCommand, we just use the command name directly as the function name. So for the disable command it's now func Disable(). I decided this was a nicer change as everything inside cmd is obvious it's for the CLI and not for anything else.

There is a new file cli.go inside cmd which is handling everything primarily cli related. Only main is used to call the root command which sets up everything else.

I've removed the checkErr(err) from each command as well, as we have RunE() functions on every command, so they always return the error and will always return back to the root command caller and print from there. This stops the issue with double printing the error text and help text unnecessarily. If we need formatted results we can just format an error and return it.

I've ensured all commits are signed off and have the correct naming, linter passed all checks locally as well.

Let me know if you would like anything amended here!

[glimchb]

I definitely like cobra.CheckErr(err) removal
I don;t see any benefit yet for the init approach... what exactly are we saving here ?
seems same amount of code... and we only have handful commands, so maintenance should not be a problem..

wdyt ?

[glimchb]

how about merging d409fb5 separately and then coming back to this PR and discussing it a little more...

[Xeckt]

I definitely like cobra.CheckErr(err) removal I don;t see any benefit yet for the init approach... what exactly are we saving here ? seems same amount of code... and we only have handful commands, so maintenance should not be a problem..

wdyt ?

I agree maintenance right now isn't a problem.

The benefit of init is just to simplify adding commands.

I wanted to set this up ahead of time, so that the process of anyone coming to the source and/or adding new commands is far more straightforward. So think of it as a future proof approach, and it's a suggested approach by Cobra.

We don't have to keep adding cmd.AddCommand(Command) in the root cobra command either when a new command is added, you do it while you make the new command. It simplifies the approach while maintaining the same functionality.

While it may not be a huge difference now, with larger sources and CLI, it definitely makes a difference down the line. I've worked on a source that had over 50 commands including subcommands, and we had implemented this approach early and it saved a lot of headache and 50+ lines of AddCommand in one place.

If we ever expand Cobra's function in sztp at a later date, it keeps the root command code meaningful rather than a load of duplicate lines.

Additionally, if we had any other command at a later point that requires more work with the root command, we can keep it relevant in the command file init rather than the root command function.

I'm more than happy to merge the error fixes for now if you prefer while we discuss.

[glimchb]

@Xeckt you made a good argument, I can merge it, please fix commitlint

[glimchb]

@Xeckt please rebase and fix commitlint

[Xeckt]

I will, I am just at work at the moment, so will do this later.

[glimchb]

@Xeckt please squash commit and fix commitlint

[Xeckt]

@Xeckt please squash commit and fix commitlint

I know, already working on it, my local git seems to have gotten all messed up, resolving the issue now.

[Xeckt]

Really weird issue. My fork was synced with main and local changes had no conflicts but kept conflicting on git. Had to rebase with the recent pushes to upstream and fix the HEAD then refix the conflicts... Should all be good now.

[glimchb]

See This branch cannot be rebased due to conflicts
I can't merge it

try

git fetch --prune --all --recurse-submodules=no --tags
git rebase origin/main

[Xeckt]

Done, thats really weird because i fixed those conficts and pushed yesterday....