grafana / shared-workflows

A public-facing, centralized place to store reusable workflows used by Grafana Labs.
GNU Affero General Public License v3.0
8 stars 14 forks source link

feat(lint-pr-title): rewrite in Typescript and handle `merge_group` events #233

Closed iainlane closed 1 month ago

iainlane commented 2 months ago

This is a big update. The initial motivation was to support merge_group events but this required some other changes.

We use merge queues in this repository, and CI is currently failing for merges because this action doesn't support the event. We can't ensure that commits going into main have messages in the correct format. We'd like to use release-please to have releases, and this requires conventional commit messages, so this lack of support is blocking us from having a proper release strategy.

When linting for a merge group event, the title is no longer the thing that you want to check. The commits themselves are what will end up in the main branch. Getting the commits which will be pushed from the merge group is a bit tricky. As for PRs, there are multiple strategies which can be used (squash, merge, rebase). There can also be multiple PRs in a merge group. The PRs to be merged might well be behind the target branch and so rebased by GitHub. We need to be able to handle all of these cases in order to lint the correct commits. Essentially we call a GitHub API to figure out what the commits in our merge group branch are. In the case there are multiple PRs in a merge group, each will get a branch based on the previous (imagine a chain). In this case we will lint each PR's commits separately, and each will get its own results. This is all covered by tests. Tests are a new addition to this action too.

Now, the final part. The Typescript rewrite. Trying to add the merge_group event without the help of types proved to be too hard. In order to get more typing support, I switched to using the upstream libraries from the commitlint project too. This broke the build. That's for one main reason: in commitlint configuration files an extends keyword can be given.

{
  "extends": ["@commitlint/config-conventional"]
}

Notice that's a string. It ends up resolving to a require call in the resulting code. node (JavaScript) Actions have to have their dependencies bundled, but the bundler has no way to know to bundle this module, and even if it did the dynamic require would still refer to a module that isn't installed, since it would be in the bundle.

To get around all of this, we're switching off the node runtime and using bun directly. This isn't detectable from the perspective of the user - good - but for working on the action it has a couple of benefits: