Improving Code Quality

[rocketeerbkw]

I'd like to start a discussion to come up with some actions to help improve the code quality in lagoon. Specifically I'm thinking about the API (and other Node.js services) but this can apply to all our code bases (go, bash, etc).

What should we be doing?

• Code style enforcement?
• Code linting?
• Typescript checking?
• Testing?

How should we be doing it?

• Just manually, left up to each dev?
• Pre-commit hook?
• For each commit/PR?

Which tools should we use?
What are we currently doing?

[tobybellwood]

Definitely keen on this - we've been tripped up a couple of times just because we don't have shellcheck running across bash scripts. Personally, I'm keener on a post-commit GitHub Action-type nudge to get people into compliance, but happy to take any recommendations

[bomoko]

My preference is for strong conventions that are enforced for each commit/PR - pre-commit hooks are okay, but depending on what they're required to do can be iffy.

As for what we should be checking:

• Code style enforcement - yes
• Code linting - yes
• Typescript checking - yes
• Testing - I'm assuming this is unit testing you have in mind?

[smlx]

I've got a few opinions on this topic, I hope you don't mind a bit of a brain dump :-)

Improve the Lagoon test suite

IMO this is the #1 problem with developing Lagoon today.

Currently we have a quite extensive integration test suite, but very little in the way of unit testing. I've seen this referred to as the test ice-cream cone[0], as opposed to the test pyramid[1].

I think we experience most of the known problems[3] of tests structured this way:

As you move higher up the agile testing pyramid, you encounter issues like:

• Test fragility (tests that break easily and unexpectedly, even when changes 'shouldn't' have influenced the test)
• Longer feedback time
• Increased effort levels
• Higher costs to implementation
• More specialized knowledge required

For example, UI tests break easily, often because the UI structure changes over time. Low-level tests are faster and more stable, so this is where we recommend you put the vast majority of your effort. Then, write very few higher-level tests, like end-to-end tests.

I'd like to see Lagoon re-architected with unit testing in mind. This means things like:

• breaking functionality out into modules
• clearly defining the interfaces of those modules
• unit testing those interfaces
• measuring test coverage

There are plenty of tools to do this, but the way Lagoon is currently architected makes it very difficult to apply those tools.

Document developer best practices

As mentioned earlier in the discussion there are LSPs and other linting / code quality tools with editor integration.

We should document how to get set up with the popular editors with at least linters for the main languages in Lagoon like shellcheck and yamllint. I'm sure there are Language Servers for the other main Lagoon languages such as typescript and javascript too.

Obviously we can't enforce how people send PRs but at least we can document a "happy path". That way people will fix style/lint issues when their editor tells them to instead of sending a PR and then having to look at CI output.

Use more CI tooling

Similarly to the previous section, we should put the same checks in CI. This will catch any issues that do slip through into a PR.

These kinds of style/lint checks should be made "required" checks in Github IMO.

Improve release engineering

The Lagoon release process is currently too manual, opaque, and slow. We should automate this more, document the parts which aren't automated, and prioritise the automation work.

Ideally this should be fully GitOps driven: merge a branch or push a tag -> get a fully usable release, including helm charts etc.

This should also have a goal of increasing the frequency of releases. Daily releases shouldn't be a big deal.

The advantage of doing this is that changes go from PR -> production much more quickly. This makes the lifetime of bugs / feature requests much shorter, as well as speeding up the feedback loop for developers.

Improve release engineering documentation

For example, here are some release engineering questions that, from what I can tell, aren't documented:

• How does a Lagoon Github Release get generated?
• How do Lagoon images get pushed to Docker Hub, and which ones specifically make up a Lagoon release? What about helm charts?
• What is the procedure for upgrading a Lagoon instance?
• Where are the release notes, and how am I notified of breaking changes?
• Does Lagoon use Semantic Versioning? If so, how is this enforced?

This is about reducing the bus factor. Maybe we can rotate Lagoon release responsibilities to a different person on each release?

Document and enforce development best practices

Some things we currently do that IMO lead to more bugs and poorer code quality:

• committing directly to main/master
• PR without filling in the template and labelling appropriately
• merge without passing CI checks, and preferably some kind of peer review
• commits/PRs that don't explain why the change is being made

I think some of this can be enforced in Github.

Rewrite anything written in bash

This one might be controversial so I left it for last, but here is my reasoning:

• bash is great for orchestrating processes, but lack of typing and very loose semantics makes it is possible to have many bugs which aren't possible in other programming languages.
• bash encourages practices like global variables which makes unit testing and maintenance more difficult.
• The development tooling for CI, editor integration, linting, and static analysis is not available for bash the way it is in other languages, so we don't pick up issues that would be easily caught in other languages.

Maybe this is more of a stretch goal :)