Conversation
| - name: Validate PR title 🔎 | ||
| uses: amannn/action-semantic-pull-request@48f256284bd46cdaab1048c3721360e808335d50 # v6 | ||
| env: | ||
| GITHUB_TOKEN: ${{ github.token }} | ||
| with: | ||
| # Require conventional commit types | ||
| types: | | ||
| feat | ||
| fix | ||
| docs | ||
| style | ||
| refactor | ||
| perf | ||
| test | ||
| ci | ||
| chore | ||
| revert | ||
| proposal | ||
| # Scope is optional | ||
| requireScope: false | ||
| # Disallow uppercase first letter in subject | ||
| subjectPattern: ^[A-Z].+$ | ||
| subjectPatternError: | | ||
| The subject "{subject}" must start with an uppercase letter. | ||
| Example: "feat: Add new component" not "feat: add new component" |
There was a problem hiding this comment.
This is a dupe of release-lint-pr-title. We could either switch that workflow to pull_request_target or we could sunset that one in favor of this one. I kind of like having that check in its own file, but I don't feel strongly either way.
| run: | | ||
| # GitHub sends CRLF bodies, which would leave a stray CR on every line and | ||
| # make an "empty" section look filled. Drop HTML comments too, so the | ||
| # template's own guidance text doesn't count as content; perl reads the | ||
| # whole body at once, which a line-based strip could not do for a comment | ||
| # spanning several lines. | ||
| text=$(printf '%s\n' "${BODY}" | tr -d '\r' | perl -0777 -pe 's/<!--.*?-->//gs') | ||
|
|
||
| if grep -qF "${HEADING}" <<<"${text}"; then | ||
| section=$(awk -v h="${HEADING}" 'index($0,h)==1 {f=1; next} f && /^###/ {exit} f' <<<"${text}") | ||
| what='The "Brief description of Pull Request" section' | ||
| else | ||
| # The template was removed. What we guard against is an empty squash | ||
| # commit message, not a missing heading, so accept prose in its place. | ||
| section="${text}" | ||
| what='The pull request description' | ||
| fi | ||
|
|
||
| if [[ -z "${section//[[:space:]]/}" ]]; then | ||
| echo "::error::${what} is empty. Alloy squashes with the pull request body as the commit message, so it needs a factual summary of what changed. See ${CONTRIBUTING}" | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "${what} is filled in." |
There was a problem hiding this comment.
This scripting scares me a bit. I kinda sorta kinda follow the logic, but it's very obtuse. If this were an internal-only tool I'd probably be more okay with it, but given that we're in sensitive territory here with respect to security, my preference would be to make the logic look as simple and straightforward as possible for human reviewers.
Using github-script is perhaps contentious in its own right, but I'd be curious of your thoughts on this sort of thing as an alternative without going to a full-blow go script for this.
- name: Check for a brief description 📝
uses: actions/github-script@v7
env:
HEADING: "### Brief description of Pull Request"
CONTRIBUTING: https://github.com/grafana/alloy/blob/main/docs/developer/contributing.md
with:
script: |
const body = (context.payload.pull_request.body ?? "")
.replace(/\r/g, "")
.replace(/<!--[\s\S]*?-->/g, "");
const heading = process.env.HEADING;
const headingOffset = body.indexOf(heading);
let description;
let label;
if (headingOffset >= 0) {
const sectionStart = headingOffset + heading.length;
const nextHeading = body.indexOf("\n###", sectionStart);
description = body.slice(
sectionStart,
nextHeading === -1 ? undefined : nextHeading,
);
label = `The "${heading}" section`;
} else {
// A PR may deliberately omit the template; require prose somewhere.
description = body;
label = "The pull request description";
}
const hasNonWhitespace = /\S/.test(description);
if (hasNonWhitespace === false) {
core.setFailed(
`${label} is empty. Alloy squashes with the pull request body as ` +
`the commit message, so it needs a factual summary of what changed. ` +
`See ${process.env.CONTRIBUTING}`,
);
return;
}
core.info(`${label} is filled in.`);| { | ||
| echo '### Unsigned commits' | ||
| echo | ||
| echo '```' | ||
| echo "${unsigned}" | ||
| echo '```' | ||
| echo | ||
| echo "Re-sign these commits in your fork and force-push. See [Signed commits](${CONTRIBUTING})." | ||
| } >>"${GITHUB_STEP_SUMMARY}" |
There was a problem hiding this comment.
If we find users get confused about trying to figure out why these checks are failing, we have some good tooling for adding issue comments, but this expands the permissions scope of the jobs and might not be worth that tradeoff.
https://github.com/grafana/alloy/blob/main/tools/release/internal/github/client.go#L331
| permissions: | ||
| pull-requests: read # List the commits on the PR. | ||
| steps: | ||
| - name: Check commit signatures 🔑 |
There was a problem hiding this comment.
Suggestion for a slight wording tweak that will show up in the checks on the PR without having to go to the job logs itself.
| - name: Check commit signatures 🔑 | |
| - name: Ensure all commits are signed 🔑 |
2 participants
Brief description of Pull Request
Add a
PR Preflightworkflow validating the pull request title, brief description, and commit signatures. It runs onpull_request_targetso these report on fork pull requests instead of waiting for a maintainer to approve the workflow run.Pull Request Details
Title linting already exists but sits behind that approval, so it ran on 14 of the 106 open external pull requests. Signing is enforced by an org ruleset that blocks the merge, while the existing
signed-commitscheck only reports.release-lint-pr-title.ymlstays until the new contexts are required.PR Checklist