KubeVirt coding guidelines or best practices

[0xFelix]

Is your feature request related to a problem? Please describe:

KubeVirt could need a coding guideline or at least best practices for the codebase, to avoid code which is harder than necessary to understand and to maintain.

Some ideas (not ordered):

• Decide on a single pointer helper package (kubevirt.io/kubevirt/pkg/pointer vs k8s.io/utils/ptr)
• Especially in test code: Passing around of virtClient vs calling kubevirt.Client() when needed
• Use the new PatchSet interface (introduced in refactor: use patch package instead of hardcoded operations #11211) instead of fmt.Sprintf to build patch sets
• Prefer switch/case instead of large if/else if/else blocks
• Prefer early returns instead of indenting/nesting if/else blocks unnecessarily deep
• Use path package to construct paths instead of using fmt.Sprintf
• Isolate code by introducing/using interfaces instead of using bare structs where possible
• Inline err checks where possible
• Avoid usage of global variables (use structs/receivers instead to keep state)
• Use constants to avoid repetition of hardcoded values
• Avoid duplication of testing code by using DescribeTables as much as possible
• Use uniform naming scheme for package imports (k8sv1 or v1, or virtv1 or v1)
• Avoid closures/ use funcs instead when not capturing any meaningful variables
• Avoid named returns
• Declare empty slices with the var syntax (pay attention when serializing data, see Declaring Empty Slices)

Ideally we could collect some more ideas and create a doc from that. Any input on that?

Describe the solution you'd like:

The outcome of this issue should be a doc containing KubeVirt coding guidelines.

Describe alternatives you've considered:

Do not provide guidelines and become desperate maintaining the code. :)

Additional context:

See work of sig/code-quality: https://github.com/kubevirt/kubevirt/pulls?q=is%3Apr+label%3Asig%2Fcode-quality+

[0xFelix]

/cc @alicefr

[0xFelix]

/sig code-quality

[alicefr]

Example of if statement simplification by using return early pattern: #11260

[fossedihelm]

I really like the Describe alternatives you've considered: section 🙂

[aburdenthehand]

/cc @aburdenthehand

[machadovilaca]

nice work, and adding to these ideas, I think that following the work done as part of kubevirt/community#219, it would also make sense to add some guidelines related to monitoring

[victortoso]

I'd like to add some line length limit! We might have wide screens in 2024 but a limit is needed :)
Just looking at pkg/

# 99 cases of lines with length between 300 and 500
toso@tapioca ~/s/k/kubevirt > grep -rx '.\{300,500\}' pkg/ | wc -l
99

# 2266 cases of lines length between 150 and 300
toso@tapioca ~/s/k/kubevirt > grep -rx '.\{150,300\}' pkg/ | wc -l
2266

And, IMHO, 150 lines should be big enough already :)
Simple example.

--

I would also suggest that, architecture related logic should be in their own code block.
Example: 15946b2

--

We should probably have a suggested indentation when a if or a function itself is too big too.

--

Last but not least, somewhat related to code guidelines is logging. We have this issue #8164 about log convention with a draft proposal #9398

Just my suggestions ;)
Cheers!

[alicefr]

One please here, if you see places in the code, where we could apply these best-practices already, please create an issue and use the label good-first-issue. Like #11305.

In this period, we can expect new contributors seeking for simple issues to solve as KubeVirt was accepted as part of GSoC this year. This kind of issues are a great start for people new to open source and kubevirt.

[fabiand]

@0xFelix Great to see this!

How do we take this forward? How about starting with a rather minimal docs/coding-conventions.md file?
Minimal in order to start with conventions which we have consensus on. And minimal in order to make it easier to remember.

Let me note that Kube also has similar files, such as https://github.com/kubernetes/community/blob/master/contributors/guide/coding-conventions.md

[0xFelix]

@fabiand Good idea, I will create a proposal PR for docs/coding-conventions.md. I expect the conventions we agree upon will be discussed in the PR.

[0xFelix]

Got a proposal for docs/coding-conventions.md posted here: #11456