CI: Add semantic versioning test for controlled breakage

[xFrednet]

This PR adds cargo-semver-checks to marker's CI, as a sanity check. This CI job shouldn't block a PR from being merged, but it's a good indicator, that a change should be listed in Marker's changelog.

Ideally, I would have liked a job status, between success and failed. Maybe a nice orange icon, saying that the job failed, but that that's acceptable? I know GitLab had something like this in the past, but GitHub doesn't seem to support it. It's either: success, failure, or cancelled. Oh well, an indicator is an indicator.

Here is an example, of a failed semver check, due to a removed public function: https://github.com/xFrednet/marker/actions/runs/6340462879/job/17221897343?pr=1

---

I should really stop procrastinating on my uni assignments... Oh well, at least this way I have the feeling I'm doing something useful.

Anyways:

Closes: #120

And anyone reading this, have a beautiful day :D

[Veetaha]

Nice quality of life improvement 👍

[Veetaha]

Let's add continue-on-error, so that if this job fails the workflow will still be displayed as green in the Actions tab of the repo.

Suggested change

	runs-on: ubuntu-latest
	if: ${{ github.event.pull_request }}
	runs-on: ubuntu-latest
	# This CI check isn't required to pass just yet.
	# Marker isn't stable so semver breaking changes are allowed.
	continue-on-error: true
	if: ${{ github.event.pull_request }}

[xFrednet]

This will require us to always look at the log, to see if the semver-check actually fails. 🤔

For a second, I contemplated if it would be cool to automatically add/remove the I-api-break label. But then I considered the implications of giving PR edit permissions to a normal user editable CI job, and decided against it...

[xFrednet]

I think having the check green by default is better than having it red 👍

Do you have another idea, how we could maybe get notified about the API breakage, without checking the job every time?

I considered checking for the I-api-break label and error if that one isn't present. However, the CI would not rerun if the label is added afterwards, resulting in a red CI again :/

The last test commit, contains some API breakage, just for a reference how it would look like.

[Veetaha]

I wouldn't put the continue-on-error on the cargo semver-checks step itself.
This way the check itself will always be green, which I suppose isn't what we want. It should be displayed as red if a major breaking change was detected.

The continue-on-error at the job level makes it so that the workflow itself is displayed as green in the github action runs like here: https://github.com/rust-marker/marker/actions

But inside the PR view the job should be displayed in red.

---

Regarding the notification. Yes! Auto-labeling the PR without granting the downstream fork broad write permissions is doable. Checkout out this blog post, it considers a similar flow of sending a comment to the PR after the tests on CI.

I think we can use the workflow_run approach where the untrusted regular CI on pull_request event runs the semver checks, and uploads the artifact with the result of the check. Then, a followup privileged CI is triggered on workflow_run event. It will run in the context of the upstream repo (and not PR merge commit), so it will not observe any PR code modifications, and thus is safe. In our case it doesn't even need to check out the PR code or even do any code checkout whatsoever.

The unprivilleged semver-checks job needs to upload an artifact with the results of the checks, and the privileged workflow downloads the artifact (docs) and make take advantage from repo write permissions.

In our case the artifact should store the PR number and the boolean result of the check so that the workflow could determine if it needs to add a PR label (if one isn't already there), or remove it (if there is one).

---

It would be ideal if cargo semver-checks supported github annotations (obi1kenobi/cargo-semver-checks-action#6), and could automatically generate notices that look like PR comments which point out to the particular file/span that triggered the check fail. Maybe if cargo semver-checks had machine-readable output (obi1kenobi/cargo-semver-checks#6) we could generate the annotations using that. It's as simple as echoing a string in specific format (docs).

[xFrednet]

Having automated GitHub annotations would be the dream! I've subscribed to the state of the issue now, but it's likely that that solution will take its time.

I've now moved the continue-on-error, what I dislike a bit, is that it marks the PR as "failed". The CI job itself was less my concern.

I think the split CI approach, with one privileged CI for applying the label, would work well for the meantime. I'll try to find some time to test this.

Thank you for the suggestion!

[Veetaha]

I think we may just merge this as is, and implement the auto-tagging workflow separately. It's better to have something than nothing, right?

[xFrednet]

You're right, I have to learn to get a better balance of what to merge or what to perfect first :)

[xFrednet]

I accidentally committed the breaking change as well 🤦