Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci(github): workflow to ensure PR description & commit message parity #2214

Closed
petermetz opened this issue Nov 22, 2022 · 2 comments · Fixed by #3436
Closed

ci(github): workflow to ensure PR description & commit message parity #2214

petermetz opened this issue Nov 22, 2022 · 2 comments · Fixed by #3436
Assignees
Labels
dependencies Pull requests that update a dependency file Developer_Experience enhancement New feature or request P4 Priority 4: Low Tests Anything related to tests be that automatic or manual, integration or unit, etc.

Comments

@petermetz
Copy link
Contributor

Description

As a maintanier/reviewer I want to have automation that makes sure that people don't just fill out the PR description on GH and leave their commit message basically empty so that the real explanation for the changes are contained in the git history and not in a database that we don't control.

Acceptance Criteria

  1. Implement the solution via a GitHub check like the PR title linting that we have in place right now.
  2. Make sure to have a manual override built-in (available only to the reviewers on a case-by-case basis) so that in special cases the check can be marked as passing (but the default is to fail the check and not let the PR get merged without the commit message being legible)
  3. Set it up in a way so that it's part of the checks executed for every pull request and it gets executed when the PR gets updated as well (e.g. if I force push something and change the commit message to something non-legible it should start failing again)
@petermetz petermetz added enhancement New feature or request dependencies Pull requests that update a dependency file Developer_Experience Tests Anything related to tests be that automatic or manual, integration or unit, etc. P4 Priority 4: Low labels Nov 22, 2022
@zondervancalvez zondervancalvez moved this from Todo to In Progress in Cacti_Scrum_Project_v2_Release Apr 25, 2024
@petermetz
Copy link
Contributor Author

petermetz commented May 2, 2024

Further context:
You can use this GitHub Action from the marketplace to make API calls directly from the workflow YAML: https://github.com/octokit/request-action

The above then enables you to retrieve the PR description and the PR commit's message through the GitHub API.

Finally with this information being available you can run a comparison and issue a warning state on a check you wrote so that people who are the authors of a pull request where the commit message and the pull request description are mismatched can get to know about this problem.

To determine the exact endpoints to use to retrieve the information above, check the GitHub REST API docs: https://docs.github.com/en/rest?apiVersion=2022-11-28

So to tie it all together:
You'd create a check job in one of our yaml files which would then run on pull requests and perform the data retrieval through the GitHub API to get the PR description and commit message, compare the two and set the status of the check to failed or succeeded based on that.

@petermetz
Copy link
Contributor Author

@zondervancalvez Some test to demonstrate what it should do on a high level:

Test case 1:

PR Description: "asdf"
Commit 1 message in the PR: "asdf"

^^ The check should pass here because everything from the PR description is present.

Test case 2:
PR Description: "asdf - some other information as well"
Commit 1 message in the PR: "asdf"

^^ The check should FAIL here because the person updated the PR description with new information that is missing from the git commit message.

Test case 3:

PR Description: "asdf"
Commit 1 message in the PR: "abcdefg"
Commit 2 message in the PR: "asdf"

^^ The check should PASS here because there was a commit in the pull request that contained the information from the PR description. It's OK if commit 1 didn't have it because at least commit 2 did so the information won't be lost.

jagpreetsinghsasan added a commit to jagpreetsinghsasan/cactus that referenced this issue Jul 17, 2024
    Primary Changes
    --------------
    1. Added pr-commit-parity.js script to check if the
       pr message and commit messages matches
    2. Updated the ci.yaml to incorporate the same

Fixes hyperledger-cacti#2214

Signed-off-by: jagpreetsinghsasan <[email protected]>
jagpreetsinghsasan added a commit to jagpreetsinghsasan/cactus that referenced this issue Jul 23, 2024
    Primary Changes
    --------------
    1. Added pr-commit-parity.js script to check if the
       pr message and commit messages matches
    2. Updated the ci.yaml to incorporate the same

Fixes hyperledger-cacti#2214

Signed-off-by: jagpreetsinghsasan <[email protected]>
jagpreetsinghsasan added a commit to jagpreetsinghsasan/cactus that referenced this issue Jul 23, 2024
    Primary Changes
    ---------------
    1. Added a script which ensures PR body and commit message
       parity

    Changes needed to incorporate 1)
    --------------------------------
    2. Added a new workflow to enable the same

Fixes hyperledger-cacti#2214

Signed-off-by: jagpreetsinghsasan <[email protected]>
jagpreetsinghsasan added a commit to jagpreetsinghsasan/cactus that referenced this issue Jul 23, 2024
    Primary Changes
    ---------------
    1. Added a script which ensures PR body and commit message
       parity

    Changes needed to incorporate 1)
    --------------------------------
    2. Added a new workflow to enable the same

Fixes hyperledger-cacti#2214

Signed-off-by: jagpreetsinghsasan <[email protected]>
jagpreetsinghsasan added a commit to jagpreetsinghsasan/cactus that referenced this issue Jul 23, 2024
    Primary Changes
    ---------------
    1. Added a script which ensures PR body and commit message
       parity

    Changes needed to incorporate 1)
    --------------------------------
    2. Added a new workflow to enable the same

Fixes hyperledger-cacti#2214

Signed-off-by: jagpreetsinghsasan <[email protected]>
jagpreetsinghsasan added a commit to jagpreetsinghsasan/cactus that referenced this issue Jul 23, 2024
    Primary Changes
    ---------------
    1. Added a script which ensures PR body and commit message
       parity

    Changes needed to incorporate 1)
    --------------------------------
    2. Added a new workflow to enable the same

Fixes hyperledger-cacti#2214

Signed-off-by: jagpreetsinghsasan <[email protected]>
@jagpreetsinghsasan jagpreetsinghsasan moved this from In Progress to In review in Cacti_Scrum_Project_v2_Release Jul 30, 2024
petermetz pushed a commit to jagpreetsinghsasan/cactus that referenced this issue Aug 8, 2024
    Primary Changes
    ---------------
    1. Added a script which ensures PR body and commit message
       parity

    Changes needed to incorporate 1)
    --------------------------------
    2. Added a new workflow to enable the same

Fixes hyperledger-cacti#2214

Signed-off-by: jagpreetsinghsasan <[email protected]>
petermetz pushed a commit that referenced this issue Aug 8, 2024
    Primary Changes
    ---------------
    1. Added a script which ensures PR body and commit message
       parity

    Changes needed to incorporate 1)
    --------------------------------
    2. Added a new workflow to enable the same

Fixes #2214

Signed-off-by: jagpreetsinghsasan <[email protected]>
@github-project-automation github-project-automation bot moved this from In review to Done in Cacti_Scrum_Project_v2_Release Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file Developer_Experience enhancement New feature or request P4 Priority 4: Low Tests Anything related to tests be that automatic or manual, integration or unit, etc.
Projects
None yet
3 participants