Static code analysis/code quality

[craigfowler]

It's been mentioned on Gitter once I think, but I'd like to introduce some static code analysis/code quality metrics into the CI process.

I'm open to suggestions as to which mechanism to use for this but I have quite a bit of experience with SonarQube and have found it rather good. SonarCloud is free for OSS projects.

Frothing about how much I like SonarQube

Here's a list of reasons I personally like SonarQube 😆

• It is built upon the principle that existing codebases might already have bad code smells, it will only fail a build if newly committed code does not meet the quality standards.
  • It will still tell you about old code that's not great but it doesn't demand you fix it all at once.
  • This means you are immediately preventing code quality from worsening and can then tackle historic issues later
• It integrates really well with the dotnet build process
• It can receive code coverage reports from coverlet, which the test projects already have installed
• The dashboard is quite pleasing to read too, "here's one I made earlier" - an OSS project of my own which uses it
• You configure "Quality Gates" (IE: how good the code has to be in order to pass).
  • Those could be quite lax at first and be tightened to improve standards over time.
  • Like, we don't have to go in right off-the-bat and say "All new code must have minimum 80% test coverage", which might not align with current project standards 😁 😉

Next steps

I'm totally open to discussion/advocacy of other analysis tools/software/strategies/ideas etc.
If we're cool with the idea of setting such a thing up then we can decide what we want to get out of it and then generate an issue to actually do that work and set it up.

[sfmskywalker]

I have no issues going forward with SonarQube. If you'd like to take the lead on this then that would be super helpful.

[craigfowler]

Something to be considering is the Quality Gate which is configured with SonarCloud. This is basically what standard the "new code" and "the project overall" needs to live upto in order to be considered a pass or a fail by SonarQube.

An aspect of that is also a Quality Profile which determines which code analysis rules are run, but the default is absolutely fine for starters. We can consider tweaking that much later, if we ever do it. I'm going to skip over that for now.

The Quality Gate config looks (for example) like this. What it's showing are the failure conditions. The build fails if any of those rows are true. This is the Sonar default you'll get right away unless it's changed.

Of all of that, the only bit which I think would be contentious is the level of test coverage it demands. Out of the box it requires 80% (on newly-submitted code), but we don't want every single build failing or else we're going to stop reading the results. That's all customisable though, so it'd be good to have a think about what you'd like as a starting level (zero is an acceptable answer!).

It can of course be changed later, so we could totally "build our way up to our aspiration".

[sfmskywalker]

Very nice. I suggest we start with zero code coverage for now, until we have reached a point where we have 80% code coverage. From that point on, we configure the Quality Gate to be the same to make sure we never go under that.

[craigfowler]

Well, we don't have to get to anywhere (on existing code) to configure the quality gate. The quality gate only applies to newly-added code. It's essentially saying "from now on, with new work, we adopt this standard", and older/existing code is not forced to conform to that standard.

[sfmskywalker]

I see. In that case let’s still keep it at 0, because I don’t want the requirement for tests to be an obstacle for folks to contribute features and fixes.

[craigfowler]

As of #665 this is now implemented and analysing the feature/elsa-2.0 branch.
The SonarCloud dashboard is at https://sonarcloud.io/dashboard?id=elsa-workflows_elsa-core