fix(dev): Add pre-commit hook for semantic commit message check

[pdecat]

This PR adds a pre-commit check using https://github.com/compilerla/conventional-pre-commit to ensure commit messages are valid.

Example usage:

# git commit -m "Fix AttributeError os.exists is not valid python attribute." --allow-empty
Terraform fmt........................................(no files to check)Skipped
Terraform wrapper with for_each in module............(no files to check)Skipped
Terraform docs.......................................(no files to check)Skipped
Terraform validate with tflint.......................(no files to check)Skipped
Terraform validate...................................(no files to check)Skipped
check for merge conflicts............................(no files to check)Skipped
fix end of files.....................................(no files to check)Skipped
trim trailing whitespace.............................(no files to check)Skipped
Conventional Commit......................................................Failed
- hook id: conventional-pre-commit
- exit code: 1

[Bad Commit message] >> Fix AttributeError os.exists is not valid python attribute.

        Your commit message does not follow Conventional Commits formatting
        https://www.conventionalcommits.org/

        Conventional Commits start with one of the below types, followed by a colon,
        followed by the commit subject and an optional body seperated by a blank line:

            fix feat docs ci chore

        Example commit message adding a feature:

            feat: implement new API

        Example commit message fixing an issue:

            fix: remove infinite loop

        Example commit with scope in parentheses after the type for more context:

            fix(account): remove infinite loop

        Example commit with a body:

            fix: remove infinite loop

            Additional information on the issue caused by the infinite loop

[pdecat]

Marked as draft as the rules for PR title validation differ from semantic commit messages, and need alignment.

[pdecat]

The remaining issue has to do with the uppercase requirement is less common AFAIK:

Error: The subject "add pre-commit hook for semantic commit message check" found in the pull request title "fix(dev): add pre-commit hook for semantic commit message check"
didn't match the configured pattern. Please ensure that the subject
starts with an uppercase character.

https://github.com/terraform-aws-modules/terraform-aws-lambda/actions/runs/8522151845/job/23341728163?pr=556

But at least, it is the same subjectPattern across all modules: https://github.com/search?q=org%3Aterraform-aws-modules+action-semantic-pull-request+subjectPattern&type=code

[pdecat]

Sadly, conventional-pre-commit does not seem to allow enforcing the same subject pattern rule, it accepts subjects with both lowercase and uppercase first letter.

[pdecat]

Hi @antonbabenko, do you think it would be acceptable to relax that PR title rule a bit to allow both upper and lower case first letter? I understand that means quite some work as it would require submitting the same change to all repositories under terraform-aws-modules to keep things consistent.

Or should we try and submit a PR to conventional-pre-commit to implement such additional rules?

FWIW: all semantic commit message examples at https://www.conventionalcommits.org/ use a lowercase first letter.

[antonbabenko]

I don't want us to change what has been (or "was") working for a long time. The PR title should be written well (I edit it now), but the commits itself can be anything.

[antonbabenko]

Let's close this PR.

[pdecat]

At least, we could keep the basic checks to avoid issues like we faced last week: #554 (comment)

[pdecat]

but the commits itself can be anything

it is the commit message that triggers the semantic release behavior

[pdecat]

I guess using the PR title as the commit message only works when squashing during PR merge, but it does not work when there's a single commit AFAIU.

[pdecat]

Apparently, there's something that can be done to avoid last week's issue:

Legacy configuration for validating single commits

When using "Squash and merge" on a PR with only one commit, GitHub will suggest using that commit message instead of the PR title for the merge commit. As it's easy to commit this by mistake this action supports two configuration options to provide additional validation for this case.

          # If the PR only contains a single commit, the action will validate that
          # it matches the configured pattern.
          validateSingleCommit: true
          # Related to `validateSingleCommit` you can opt-in to validate that the PR
          # title matches a single commit to avoid confusion.
          validateSingleCommitMatchesPrTitle: true

However, GitHub has introduced an option to streamline this behaviour, so using that instead should be preferred.

https://github.com/amannn/action-semantic-pull-request?tab=readme-ov-file#legacy-configuration-for-validating-single-commits

[antonbabenko]

I think I clicked the wrong button last week when merging PR using the GitHub Mobile app. I have just reviewed the settings for this repository to make sure that we always "Squash and merge". Thanks for the follow-up!

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.