move from SonarCloud to reporting code quality and coverage through GitHub Actions

[egpbos]

GEMDAT uses GitHub Actions to generate a coverage badge, which is published to a GitHub Gist and from there can be included in the README.md. See https://github.com/GEMDAT-repos/GEMDAT/blob/main/.github/workflows/tests.yaml

Something like this could also perhaps be done for code quality.

This way, we could remove SonarCloud as an external dependency, which would simplify setup. Ideally, the action would be published separately, so that not every generated package has to maintain this workflow themselves, but that could be a next step as well.

It would also fix the issue in #221, because creating a badge doesn't have to be done in a PR, and that's the only place where the secret is necessary in the workflow.

@v1kko or @stefsmeets would you be interested in (helping) contributing this?

[stefsmeets]

Yeah, I can have a go at this.

[stefsmeets]

Hi @egpbos I added the workflow in the PR. I'm not sure if this fixes #221, because a user will still need to edit the workflow and add their gist ID before the workflow passes.

[bouweandela]

In the guide we recommend using a code coverage service, so we should do the same in the template. A badge is nice for potential users so they can quickly get an impression of the quality of the software, but it lacks the detail required for a healthy development process. Code quality services provide their own badge with overall coverage on it, so I don't really see the added benefit of a custom workflow to compute that.

[egpbos]

Hrm, good points @bouweandela. Interactively browsing the code annotated with (lack of) coverage is of course a big benefit of those services.

[stefsmeets]

The same can of course be easily achieved by running this tool locally rather than to rely on a CI to do this for you. This is my preferred way, because it allows me to iterate faster. I rarely looked at sonarcloud when I still used it and found it to be a nuisance.

Obviously, there is no added benefit of using a custom badge as proposed in #370 other than to reduce your dependency on third-party services.

If you think it is better to promote the use of sonarcloud that also works for me. Feel free to close this issue and PR then.

[bouweandela]

I agree that it is more convenient to look at code coverage locally when developing code yourself, but for reviewing pull requests, it is much more convenient to look at CI. A code coverage service will also alert you of lines of code with changed coverage that might be unexpected (for example, this can happen when you refactor and remove some code and associated tests that used code that is shared with other parts of the code, but then is no longer tested). That is why we recommend running the unit tests in CI and using a code coverage service such as Codecov or Coveralls in the Guide.

From my point of view, Sonarcloud can be dropped because it is not based on the ruff linter settings that we use in the template for linting, but rather on Sonarcloud's own linter rules, which provides an inconsistent development experience.

[v1kko]

Would it be possible to put the html output of a coverage tool (something like coverage report html) into the documentation? In this way it can be accessed without needing a third party tool.

disclaimer: I have never done something like that, so I am not certain on the amount of work required

[v1kko]

Seems options already exist to do it in pull requests:

• https://github.com/barecheck/code-coverage-action (Seems very complete)
• https://github.com/marketplace/actions/coverage-comment (A lightweight example)

[egpbos]

These look very nice, especially Barecheck. They don't offer a full replacement of an interactively browseable site (yet), though. The code review annotations look nice, but also a bit confusing ("24 line is not covered with tests"? Are 24 lines not covered? Or only line 1 in that screenshot? It's not clear to me...), but let's keep watching it, maybe it will get better.

Btw, the feature they promote that "Instead of showing you all uncovered files Barecheck show only the ones you have changed." actually makes it impossible to catch cases that @bouweandela mentioned: "when you refactor and remove some code and associated tests that used code that is shared with other parts of the code, but then is no longer tested". Hopefully that will become optional in the future.