Skip to content

ci: read the commit message from env in the doc-only check - #1170

Open
kobihikri wants to merge 1 commit into
PDFMathTranslate:mainfrom
kobihikri:ci/commit-message-via-env
Open

ci: read the commit message from env in the doc-only check#1170
kobihikri wants to merge 1 commit into
PDFMathTranslate:mainfrom
kobihikri:ci/commit-message-via-env

Conversation

@kobihikri

Copy link
Copy Markdown

Hi, and thanks for PDFMathTranslate.

In .github/workflows/python-publish.yml, the "Check if commit is doc-only" step reads the commit message by interpolation:

run: |
  MSG=$(echo "${{ github.event.head_commit.message }}" | head -n 1)
  if [[ "$MSG" == doc:* ]] || [[ "$MSG" == "doc("* ]]; then

Actions expands ${{ ... }} into the script text before bash runs, so the commit message becomes part of the command rather than input to echo. A commit message is free text — it comes in with whatever a contributor wrote in the PR they got merged — and $(...) and backticks still run inside double quotes.

This is the "Test and Release" workflow, which is why I thought it worth raising rather than leaving be. It is on: push to main/master, so a message only gets here after someone merged it, and the reviewer is looking at the diff rather than the commit text.

The change reads the message from the environment:

env:
  COMMIT_MESSAGE: ${{ github.event.head_commit.message }}
run: |
  MSG=$(printf '%s' "$COMMIT_MESSAGE" | head -n 1)

I used printf '%s' rather than echo so a message beginning with something like -n is not treated as a flag; the head -n 1 and both doc: / doc( prefix tests are unchanged, so doc-only detection behaves exactly as before.

The same line appears in fork-test.yml. I have kept this PR to python-publish.yml so it stays easy to review, and I am happy to send the same change for the other one if you would like it.

Disclosure: I used AI assistance to help spot this and prepare the change, and I read both workflows myself.

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.

1 participant