fix: merge extra-options with fixed base/head PR SHAs#35
Conversation
njjetha
force-pushed
the
commit-msg-check
branch
from
4f5d6c6 to
fa9cd6f
Compare
njjetha
force-pushed
the
commit-msg-check
branch
from
fa9cd6f to
f83384f
Compare
njjetha
force-pushed
the
commit-msg-check
branch
from
f83384f to
1e0072c
Compare
| enable-commit-email-check: true | ||
| enable-commit-msg-check: false # change to true to see some commit message check failures | ||
| commit-msg-check-extra-options: '{"check-blank-line": false}' | ||
| enable-commit-msg-check: true # change to true to see some commit message check failures |
There was a problem hiding this comment.
can we make this change in the README file as well please
There was a problem hiding this comment.
As we already mentioned in README checker is optional its repo maintainer responsibility whether they want to enable it for there repo or not. We enable it for testing purpose in test-preflight.yml file.
| EXTRA_OPTIONS: ${{ inputs.extra-options }} | ||
| run: | | ||
| FIXED='{"base":"${{ github.event.pull_request.base.sha }}","head":"${{ github.event.pull_request.head.sha }}"}' | ||
| MERGED=$(printf '%s\n%s' "$EXTRA_OPTIONS" "$FIXED" | jq -sc '.[0] * .[1]') |
There was a problem hiding this comment.
If i dont give any extra options parameter it should fall back to default options. I tried testing this and it failed with this error:
@njjetha could you please check?
There was a problem hiding this comment.
@ynancher I provided no extra-options it is taking default options
https://github.com/qualcomm/qcom-reusable-workflows/actions/runs/24497250895/job/71594992739
Check the changes https://github.com/qualcomm/qcom-reusable-workflows/pull/35/changes#diff-f63eb61e18c2ea800fd5c5855ed5cfd9f3cf45c2b1bc434f0483a24da12bc511R22
You need to set the value like this commit-msg-check-extra-options: '{}'
Although action failed because sub-char-limit is 50 and commit subject is more than 50 characters.
There was a problem hiding this comment.
I understand the current behavior. However, is it possible for the checker to gracefully handle the case where the extra-options input is not provided, rather than failing?
Since the workflow already defines default values for this input, a user who is satisfied with the defaults should not be required to explicitly pass the parameter with an empty value.
There was a problem hiding this comment.
If you are not passing any extra options by default it will take {} https://github.com/qualcomm/qcom-reusable-workflows/pull/35/changes#diff-60905c0a7c412a300160ba52994487b4a3171250dfbfcd33ab38e4e2b9efcf7cR8
There was a problem hiding this comment.
But it is not taking the input as {} during execution:
Inputs
extra-options:
It is parsing it as null
There was a problem hiding this comment.
There was a problem hiding this comment.
Ahh, I got it https://github.com/qualcomm/qcom-reusable-workflows/blob/main/.github/workflows/reusable-qcom-preflight-checks-orchestrator.yml#L37
Basically it set the value to default empty string due to which it is failing. I fixed that in commit a0b208a
There was a problem hiding this comment.
I've tried out the changes and it seems to be working now.
njjetha
force-pushed
the
commit-msg-check
branch
4 times, most recently
from
bb42821 to
1e0072c
Compare
| - name: Prepare action options | ||
| id: prepare-options | ||
| env: | ||
| EXTRA_OPTIONS: ${{ inputs.extra-options }} |
There was a problem hiding this comment.
making this EXTRA_OPTIONS: ${{ inputs.extra-options || '{}' }}
will help us solve this issue, even in cases where the user hasn't explicitly added the extra options parameter in the preflight-checks-orchestrator.yml file
There was a problem hiding this comment.
There was a problem hiding this comment.
I am not sure why your action is failing but when I set commit-msg-check-extra-options: '{}' it is passing successfully you can check my workflow is successful.
https://github.com/qualcomm/qcom-reusable-workflows/actions/runs/24519688177/workflow
I can add the fallback ${{ inputs.extra-options || '{}' }} as an extra safety measure.
While callers who don't need extra options will simply omit the commit-msg-check-extra-options input entirely (relying on the declared default: '{}'), the || '{}' guards against the edge case where a caller explicitly passes an empty string ""
njjetha
force-pushed
the
commit-msg-check
branch
from
1e0072c to
18286ff
Compare
Add a prepare step that merges caller-supplied extra-options JSON with the fixed base and head values derived from the pull_request event. Use jq -sc to produce compact single-line output required by GITHUB_OUTPUT, and pass extra-options via env to prevent shell injection. Signed-off-by: Neeraj Jetha <njetha@qti.qualcomm.com>
njjetha
force-pushed
the
commit-msg-check
branch
2 times, most recently
from
8668a88 to
31d2af8
Compare
Add the `|| '{}'` fallback to the extra-options expression to guard
against the edge case where a caller explicitly passes an empty string
"" as the input value.
Signed-off-by: Neeraj Jetha <njetha@qti.qualcomm.com>
njjetha
force-pushed
the
commit-msg-check
branch
from
31d2af8 to
a0b208a
Compare
4 participants
Summary
Add a prepare step that merges caller-supplied extra-options JSON with the fixed base and head values derived from the pull_request event.
Use jq -sc to produce compact single-line output required by GITHUB_OUTPUT, and pass extra-options via env to prevent shell injection.