Normalize CLA comment text to handle trailing punctuation and multiline comments#45
Normalize CLA comment text to handle trailing punctuation and multiline comments#45
Conversation
9d28680 to
6a0f524
Compare
|
✅ All required contributors have signed the F5 CLA for this PR. Thank you! |
There was a problem hiding this comment.
Pull request overview
This pull request enhances the F5 CLA workflow to handle trailing punctuation in user comments. Previously, the workflow used exact string matching which caused CLA approvals to fail when users naturally added punctuation (e.g., a period) to the end of their agreement statement.
Changes:
- Added a normalization step that strips leading/trailing whitespace and trailing punctuation before matching CLA agreement text
- Restructured the workflow conditional to check event type before accessing step outputs, preventing undefined reference errors
- Updated documentation in README and CHANGELOG to describe the normalization behavior
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| .github/workflows/f5_cla.yml | Added normalization step with secure input handling and restructured conditional logic |
| README.md | Added note explaining automatic comment normalization |
| CHANGELOG.md | Documented the enhancement in the Unreleased section |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.github/workflows/f5_cla.yml
Outdated
| # Remove leading/trailing whitespace and trailing punctuation marks | ||
| NORMALIZED=$(printf '%s\n' "$COMMENT_BODY" | sed 's/^[[:space:]]*//; s/[[:space:]]*$//; s/[.!?;,]*$//') |
There was a problem hiding this comment.
The sed command only processes the first line of multiline comments. If a user posts a multiline comment where the CLA agreement text is not on the first line, or if there are line breaks, the normalization may fail or produce unexpected results. Consider using a different approach that handles multiline input, such as using tr to remove newlines before processing, or using sed with appropriate flags to handle multiline text.
| # Remove leading/trailing whitespace and trailing punctuation marks | |
| NORMALIZED=$(printf '%s\n' "$COMMENT_BODY" | sed 's/^[[:space:]]*//; s/[[:space:]]*$//; s/[.!?;,]*$//') | |
| # Remove leading/trailing whitespace and trailing punctuation marks, and handle multiline comments | |
| NORMALIZED=$(printf '%s\n' "$COMMENT_BODY" | tr '\n' ' ' | sed 's/^[[:space:]]*//; s/[[:space:]]*$//; s/[.!?;,]*$//') |
There was a problem hiding this comment.
Fixed in f366b11. Now using tr '\n' ' ' to convert newlines to spaces before processing with sed, which properly handles multiline comments.
.github/workflows/f5_cla.yml
Outdated
| # Normalize the comment by trimming whitespace and removing trailing punctuation | ||
| # Remove leading/trailing whitespace and trailing punctuation marks | ||
| NORMALIZED=$(printf '%s\n' "$COMMENT_BODY" | sed 's/^[[:space:]]*//; s/[[:space:]]*$//; s/[.!?;,]*$//') | ||
| echo "normalized=$NORMALIZED" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
If the normalized comment contains special characters like single quotes, double quotes, or newlines, writing it directly to GITHUB_OUTPUT may cause parsing issues or unexpected behavior. Consider using a delimiter-based multiline output format as recommended by GitHub Actions documentation, or ensure the output is properly escaped.
| echo "normalized=$NORMALIZED" >> "$GITHUB_OUTPUT" | |
| { | |
| echo 'normalized<<EOF_NORMALIZED' | |
| printf '%s\n' "$NORMALIZED" | |
| echo 'EOF_NORMALIZED' | |
| } >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
Fixed in f366b11. Now using the delimiter-based multiline output format with heredoc syntax as recommended by GitHub Actions documentation.
…nses Co-authored-by: alessfg <15654525+alessfg@users.noreply.github.com>
Co-authored-by: alessfg <15654525+alessfg@users.noreply.github.com>
Co-authored-by: alessfg <15654525+alessfg@users.noreply.github.com>
…polation Co-authored-by: alessfg <15654525+alessfg@users.noreply.github.com>
…ondition Co-authored-by: alessfg <15654525+alessfg@users.noreply.github.com>
Co-authored-by: alessfg <15654525+alessfg@users.noreply.github.com>
Co-authored-by: alessfg <15654525+alessfg@users.noreply.github.com>
6a0f524 to
2b57982
Compare
|
@copilot this normalization should only run after the contributor assistant step has run for the first time, and should only run if the user has typed in "I have hereby read the F5 CLA and agree to its terms." in response to the CLA trigger. |
Co-authored-by: alessfg <15654525+alessfg@users.noreply.github.com>
The normalization step only runs on |
Proposed changes
CLA approvals were failing when users added trailing punctuation to their agreement comment (e.g.,
"I have hereby read the F5 CLA and agree to its terms."). The workflow used exact string matching which rejected natural variations.This PR adds a normalization step that preprocesses user comments before matching them against the expected CLA agreement phrases, making the workflow more flexible while maintaining security.
Changes:
issue_commentevents to strip leading/trailing whitespace and trailing punctuation (.!?;,) before matchingtrto convert newlines to spaces, allowing the CLA agreement text to be properly matched even when split across multiple linespull_request_targetevents first, then normalized output forissue_commentevents (avoids accessing undefined step outputs)printfinstead of direct interpolation to prevent shell injectionImplementation:
Backward compatible—comments without punctuation continue to work unchanged.
Checklist
Before creating a PR, run through this checklist and mark each as complete:
README.mdandCHANGELOG.md).Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.