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

Validate pr title for traceability markers (infra) #732

Merged
merged 2 commits into from
Sep 18, 2023
Merged

Conversation

Hook25
Copy link
Collaborator

@Hook25 Hook25 commented Sep 18, 2023

Description

We have a new traceability system for PR titles but we have no way to actually enforce it right now beside manually checking.

This changes that by adding a new GH Action that validates the title to check that it actually ends with a traceability marker. To make them not too invasive and hard on contributors, these markers are case insensitive and optionally enclosed in brackets.

Resolved issues

N/A

Documentation

N/A

Tests

This PR was created with a title that does not contain any marker. The job should fail. Once that happens it will be edited to the correct title and it should pass.

Failure: https://github.com/canonical/checkbox/actions/runs/6223347828/job/16889096967?pr=732
Passing: https://github.com/canonical/checkbox/actions/runs/6223329257/job/16889036066?pr=732

@Hook25 Hook25 requested a review from kissiel September 18, 2023 13:31
@Hook25 Hook25 self-assigned this Sep 18, 2023
@Hook25 Hook25 changed the title Validate pr title for traceability markers Validate pr title for traceability markers (infra) Sep 18, 2023
@Hook25 Hook25 changed the title Validate pr title for traceability markers (infra) Validate pr title for traceability markers Sep 18, 2023
@Hook25 Hook25 changed the title Validate pr title for traceability markers Validate pr title for traceability markers (infra) Sep 18, 2023
Copy link
Contributor

@kissiel kissiel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote a simple test for this and I don't know if this is the result we want.

#!/bin/bash

# Array of test values for PR_TITLE
declare -a PR_TITLES=("infra" "bugfix" "new" "breaking" "my new pr (fix)"
                      "Infra?" "Bugfix?" "New?" "Breaking?"
                      "Some random title" "another INFRA" "BREAKING change"
                      "Bugfix at the end?" "Some more texts after breaking?"
                      "Infra!" "bugfix:" "New." "New stuff I'm adding" "traliling space after fix "
                      "fix foo infra" "change bar bugfix" "update baz new" "modify qux breaking"
                      "fix foo Infra?" "change bar Bugfix?" "update baz New?" "modify qux Breaking?"
                      "fix foo (infra)" "change bar (bugfix)" "update baz (new)" "modify qux (breaking)"
                      "fix foo (Infra)?" "change bar (Bugfix)?" "update baz (New)?" "modify qux (Breaking)?")

# Loop over each PR_TITLE and apply the grep command
for PR_TITLE in "${PR_TITLES[@]}"; do
    if echo "$PR_TITLE" | grep -iP "(infra|bugfix|new|breaking)\)?$"; then
        echo "[MATCH] $PR_TITLE"
    else
        echo "[NO MATCH] $PR_TITLE"
    fi
done

Are we not requiring ()?
Moreso, the typical my new pr (fix) marker wouldn't work.

@Hook25
Copy link
Collaborator Author

Hook25 commented Sep 18, 2023

I have made the brackets compulsory.

As for (fix) no, it is not in our convention right now, we could map it to bugfix and accept it but I think it would become a bit weird. Afterall a fix may be backward compatible, non backward compatible or relative to infra. I'm more on the side of not accepting (fix) as a valid marker

@Hook25 Hook25 requested a review from kissiel September 18, 2023 15:05
Copy link
Contributor

@kissiel kissiel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry, my mistake. Please ignore my input on "fix", "bugfix" is ok.
I've used "fix" so many times before that it's inprinted in my brain.

Overall +1

@Hook25 Hook25 merged commit 433f219 into main Sep 18, 2023
3 checks passed
@Hook25 Hook25 deleted the validate_pr_title branch September 18, 2023 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants