oursky/gitlint-bot

gitlint-bot

gitlint-bot is a Github App that lints commit messages against custom rules and periodically posts a summary of these analyses in a Slack channel.

The goal of this App is to promote better and more consistent commit message style among developers in an organization.

This Github App is built using Probot

Development Setup

Requires Docker v19.03.0+

cp .env.example .env
docker-compose up

Navigate to http://localhost:3000 and click on the Register Github App button.

Publishing Packages

In order to publish a package, bump the package version and run npm run publish. A prompt will be shown for confirmation before the packages are published.

Data-flow Diagram

License

Apache 2.0

gitlint-bot's Issues

Setup logging

• Log unhandled errors
  • In web-hook handlers (use Sentry?)
  • In cron job
• Log received commits
  • commit org/repo
  • commit hash

Duplicate Users in staging DB

In the staging db, there are some duplicate User email and name records.

I believe this issue is caused by the async execution of the list of saveCommit promises in src/listeners.ts.

Might also want to add a unique constraint on the email, name columns.

Process commit data and add to postgres

After linting the commit messages from a Push Webhook event, create Postgres records for each commit and commit diagnosis.

If a User record for the commit's committer doesn't exist, then create a new record.

Update Sentry DSN

[Web UI] List of analyzed commits

• List of all analyzed commits
  • TODO: fields to display in list page
• With pagination
• Links to detail page of the commit
  • Occurred errors during commit analysis
  • TODO: fields to display in detail page

Add commit rules

• Rules are going to be implemented as a list of rule configuration objects which contain the following fields
  • name (rule ID), valid boolean, check function
  • Check function takes the parsed commit message as an argument and might return an object like this: { valid: false, data: { ... } }
  • A CommitDiagnosis containing the check data is created for invalid commit diagnoses.
• Each rule's score is only added to the commit's overall score if the rule check is valid

[Web UI] Summary dashboard

A dashboard to display summary of the watched commits:

• per repository summary
  • most recent N bad commits
  • percentage of bad commits

Add new lint rules

I'm thinking of adding new lint rules to the default preset, based off the built-in rules of gitlint.

Here are the ones I'm considering:

• subject-no-trailing-whitespace
• subject-no-hard-tab: cannot contain the \t character
• replace subject-no-end-period with subject-no-trailing-punctuation
• body-max-line-length: lines in the body must be < 80 characters
• body-no-trailing-whitespace

Analyze commit

• Check commit message
  • Research tools/libraries for this
    • gitlint/commitlint
  • Rules
    • Title < 50 characters
    • No period at end of line
    • Issue tracker references in correct position
    • other sensible rules

Knex.js `returning()` method returns an array

The .returning method always returns an array of values: https://knexjs.org/#Builder-returning

The TypeScript types for the library do not accurately show this return type. In the db/index.ts module, the returned array is being accessed like a model object which causes some fields to be undefined.

Add new commit model columns

Including the following information about each commit would help the summary messages:

• Repo name
• Branch name/commit reference

[Web UI] Tracking issue

A simple web UI to display information that cannot fit within the weekly Slack message.

A frontend framework/good visual design is not required, but clean HTML with correct semantics is preferred. Server-side rendered pages with templates are acceptable.

• Authentication (#60)
• Dashboard (#58)
• Commit list (#59)

Regex matching rule

Some projects use custom commit message convention, such as:

ci: Add deploy notification

It triggers false positive on rule subject-capitalize-first. We may want to provide a generic regex rule to match for this case, e.g. :

rules:
  match-regex: [on, null, "^\p{Ll}+: \p(Lu}.+$"]

Project overview

• The bot can be configured to watch for:
  • repositories within specific GitHub organizations; and
  • specific GitHub repositories.
• Periodically (configurable), the commits in configured repositories
  would be analyzed for:
  • commit message quality (linting)
• The analyzed result of commits should be retrievable for subsequent lookup.
• Each commit would be given a score, indicating how bad it is.
• At the end of collection period, the analyzed commits would be summarized as:
  • the top 5 repository with highest total commit score.
  • the top 5 committer with highest total commit score.
  • the top 10 commit with highest commit score.
• The summary would be delivered as a message to a configured Slack channel (#4)
• Error tracking and logging with Sentry (#27)
• Deployed to internal k8s cluster (#5)

Notification message

• Notify weekly aggregated analyze result
• Slack webhook

Setup database

• Choose a ORM/query builder/use raw SQL
• Schema (ER diagram)
  • in README
  • can use MermaidJS
• Migrations

Fix not setting session cookies correctly

ref: https://stackoverflow.com/questions/27637609/understanding-passport-serialize-deserialize

Create helm chart

CLI tool

We should have a CLI tool that developers can run locally to produce same diagnosis (as in the bot) using configuration file.

Configuration file

Lint rules should be configurable via a configuration file stored in each repo. We may have a global default config, merged with the repo specific config.

Support multiple header patterns

Some people may want to use [scope] description instead of conventional type(scope): description. We may want to also match that in default commit header pattern.

Adjust default rule set

• Set maximum length to 80

Slack command not using default value

When the /gitlint-summary command is used without any arguments, a message for '0 days' is produced. We should be setting a default value according to environment variable, but it's not working now.

Fix wildcard globs in npm scripts

NPM runs commands using local shell (/bin/sh) for Mac which doesnt understand globbing with wildcards.

Improve notification message

Currently, the notification message is like:

In the past 7 days, there were a total of 8 analyzed commits with 0 rule violations

There is insufficient detail to be useful. We should include more information to make it useful to the users.

Slack slash command to produce summary on-request

Deployment pipeline

• On master branch update: deploy to internal dev cluster

Add tests

Setup repo

• NodeJS + TypeScript
• eslint + prettier
• CI checks using GitHub Action
• docker-compose.yaml for spinning up local dev environment.
  • Postgres

[Web UI] Authentication

• Authentication to web UI
• SSO through Google/GitHub

GitHub integration

• https://github.com/probot/probot
• Receive events on new commits to repo branches
• Analyze commit (#13)
• Add commit data to datastore (#16)
• Add tests (#15)
• save to data store

Improve image configuration

• Set Pod resource request & limit (ref: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/)
• Reduce image size (currently 371MB)

Add data flow diagram to README

Since this project involve external systems (GitHub, Slack). It would be beneficial to have a high-level DFD indicating how data flows through the system.

Update README

• Add project introduction (objective)
• Setup/installation instruction
• Architecture (i.e. DFD)
• License (Apache)
• Move ER diagram to a markdown document in docs directory.

Support whitelisting commit message patterns

Some auto-generated commits may have commit message with incorrect format. For example:

• GitBook: [master] 3 pages modified
• registered in blackbox: secrets.yaml

We may allow configuring a commit message whitelist for these commits.

Refine public API typing

Currently, the public API typing depends on external libraries used in implementation (Joi, js-yaml), we should decouple from them and have a well-defined public API.

Improve database handling

We use Promise.all for some database operations, which is not ideal:

• statements within a transaction can be run sequentially only in single connection, so there's there's no point in making them concurrent.
• Promise.all would reject once any of the promise rejected, even if other promise is still running. In some database libraries, this can lead to some statements running after the transaction rollback.

Let's use a simple loop for these cases.