new project code layout

[bmeneg]

In #552 I've started a change to help separate common code/data for different commands from code command-specific. However, this is the first step, where common code was placed in a simple *_common.go file. The project is growing somewhat quick and having a better project structure would be really helpful and important to keep developers/contributors sane :).

The repo https://github.com/golang-standards/project-layout presents one project layout that has been adopted by other projects and also points to some examples and best practices material. IMHO it seems a good starting point for lab.

Today, we have all commands and their respective subcommands placed in a one-level cmd/ folder, with partial common code being placed in the internal/ folder. This isn't wrong, but doesn't scales well, as we have started to see.

One idea, proposed by @zaquestion, is: split commands in a second level folder inside cmd/ and make them separated packages (please, correct me if I'm wrong):

cmd/
    - root.go
    - version.go
    - ...
    - mr/
        - mr.go
        - <mr common code>.go
        - list.go
        - edit.go
        - show.go
        - ...
    - issue/
        - issue.go
        - <issue common code>.go
        - list.go
        - ...
    - ... 

While common code, like the one present in cmd/util.go and *_common.go would be moved to internal/.

Another similar idea I had was to place subcommands of, for example, mr/ inside a third level cmd/ folder, so common code for different mr subcommands could be placed in the higher-level folder (I stole this approach from kubernetes project :-) ), however, in this case, the subcommands (list, edit, ...) would use the same command (mr, issue, ...) package:

cmd/
    - root.go
    - version.go
    - ...
    - mr/
        - mr.go
        - <mr common code>.go
        - cmd/
            - list.go
            - edit.go
            - show.go
            - ...
    - issue/
        - issue.go
        - <issue common code>.go
        - cmd/
            - list.go
            - ...
    - ... 

And common code for all commands moved to internal/, like the first idea.

I'm not an expirienced Go programmer, so please, any feedback is really welcome. Also, these changes will touch every single piece of code in the entire project, and because of that we must have a single and entire milestone dedicated for that IMHO. It's feasible, but also painful. So, feel free to say how worth these changes really are, suggest other ideas/approaches, ... whatever.

Many thanks.

[prarit]

Would there be a top-level common.go as well for code that applies to both commands?

[bmeneg]

Would there be a top-level common.go as well for code that applies to both commands?

Yes, there would/could also have something like that. Another thing would be to split cmd/util.go in specific files like: cmd/args.go to handle argument parsing, internal/util.go to place those functions not really related to the commands (e.g. isOutputTerminal()).

[zaquestion]

@bmeneguele @prarit Consider either of these restructures "blessed" :)

I do see a potential cyclic imports issue in the later suggestion which I'm not sure how kubernetes works around (@bmeneguele can you point to where you saw this pattern exactly)

Not sure how you get around issue.go

import "github.com/zaquestion/lab/cmd/issue/cmd" // imported to reference / wireup sub commands

cmd/list.go

import "github.com/zaquestion/lab/cmd/issue" // imported to reference common code and parent command

Additionally each command will be importing a cmd package, which could get a tad confusing or lead to package renaming for example probably want to avoid

import "github.com/zaquestion/lab/cmd/issue/cmd"
import rootCmd "github.com/zaquestion/lab/cmd"

Ideally we restructure in such a way that no imports from the root cmd package are required period

[bmeneg]

@zaquestion sorry for the late reply. But your concerns are indeed something to look at. To be honest, I didn't really dig deep enough on how Kubernetes structure/use it to avoid such issues. This example I took from their kubeadm command.

Initially, I'm far more comfortable working with the first approach than the second, which seems to add too much unnecessary overhead for what we really need.