Skip to content
This repository was archived by the owner on Sep 24, 2025. It is now read-only.

chore (ci): improve changelog check#52

Merged
dbarrosop merged 7 commits into
mainfrom
changelog
Aug 15, 2025
Merged

chore (ci): improve changelog check#52
dbarrosop merged 7 commits into
mainfrom
changelog

Conversation

@dbarrosop

@dbarrosop dbarrosop commented Aug 15, 2025

Copy link
Copy Markdown
Member

PR Type

Other


Description

  • Improve changelog validation in CI workflow

  • Change from diff-based to first-line checking

  • Add test files for validation testing

  • Update error messages for clarity


Diagram Walkthrough

flowchart LR
  A["CI Workflow"] --> B["Check PR Title Format"]
  B --> C["Extract Expected Entry"]
  C --> D["Check First Line of CHANGELOG.md"]
  D --> E["Validate Entry Matches"]
Loading

File Walkthrough

Relevant files
Enhancement
wf_check_changelog.yaml
Replace diff-based with first-line changelog validation   

.github/workflows/wf_check_changelog.yaml

  • Remove git diff-based changelog checking logic
  • Replace with direct first-line validation of CHANGELOG.md
  • Update error messages to reflect new validation approach
  • Simplify validation by checking exact match of first line
+7/-8     
Tests
CHANGELOG.md
Add test entry for validation                                                       

packages/nhost-js/CHANGELOG.md

  • Add test entry "updated" as first line
  • Used for testing the new validation logic
+2/-0     
README.md
Add temporary test content                                                             

packages/nhost-js/README.md

  • Add test line "DELETE THIS LINE"
  • Used for testing purposes during development
+2/-0     

@github-actions

github-actions Bot commented Aug 15, 2025

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 886ff0b)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Syntax Error

Missing closing brace in the GitHub Actions variable reference which will cause the workflow to fail

echo "PR title with PR number found as first line in ${{ inputs.PROJECT_PATH}}/CHANGELOG.md"
Test Pollution

Test content added to production changelog file that should be removed before merging

updated
Test Pollution

Test content added to production README file that should be removed before merging

DELETE THIS LINE

@github-actions

github-actions Bot commented Aug 15, 2025

Copy link
Copy Markdown

PR Code Suggestions ✨

Latest suggestions up to 886ff0b
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Use robust string comparison method

The string comparison using = is fragile and may fail due to whitespace differences
or special characters in the PR title. Use a more robust comparison method that
handles potential formatting issues.

.github/workflows/wf_check_changelog.yaml [55-61]

-if [ "$FIRST_LINE" = "$EXPECTED_ENTRY" ]; then
+if echo "$FIRST_LINE" | grep -Fxq "$EXPECTED_ENTRY"; then
   echo "PR title with PR number found as first line in ${{ inputs.PROJECT_PATH}}/CHANGELOG.md"
 else
   echo "::error::Expected entry '$EXPECTED_ENTRY' not found as the first line in ${{ inputs.PROJECT_PATH }}/CHANGELOG.md"
   echo "::error::Please add your PR title with PR number as the first line in CHANGELOG.md (Example: 'feat (myservice): something something #$PR_NUMBER')"
   exit 1
 fi
Suggestion importance[1-10]: 5

__

Why: The suggestion improves robustness by using grep -Fxq instead of direct string comparison, which could handle edge cases better. However, the current = comparison is standard and appropriate for exact string matching in shell scripts.

Low

Previous suggestions

Suggestions up to commit dc3506f
CategorySuggestion                                                                                                                                    Impact
General
Validate changelog entry content format

The current check only verifies if CHANGELOG.md was modified but doesn't validate
the content format. This could allow PRs with incomplete or incorrectly formatted
changelog entries to pass validation.

.github/workflows/wf_check_changelog.yaml [47]

-if git diff --name-only origin/${{ github.base_ref || 'main' }} | grep -q "${{ inputs.PROJECT_PATH }}/CHANGELOG.md"; then
+# Get the diff for CHANGELOG.md
+git diff origin/${{ github.base_ref || 'main' }} -- ${{ inputs.PROJECT_PATH }}/CHANGELOG.md > changelog_diff.txt
 
+if [ -s changelog_diff.txt ]; then
+  # Extract PR number
+  PR_NUMBER="${{ github.event.pull_request.number }}"
+  
+  # Create the expected CHANGELOG entry format with PR title and number
+  EXPECTED_ENTRY="$PR_TITLE #$PR_NUMBER"
+  
+  # Check if PR title with PR number appears in the diff
+  if grep -q "$EXPECTED_ENTRY" changelog_diff.txt; then
+    echo "PR title with PR number found in ${{ inputs.PROJECT_PATH}}/CHANGELOG.md diff"
+  else
+    echo "::error::Expected entry '$EXPECTED_ENTRY' not found in ${{ inputs.PROJECT_PATH }}/CHANGELOG.md diff"
+    echo "::error::Please add your PR title with PR number to CHANGELOG.md (Example: 'feat (myservice): something something #$PR_NUMBER')"
+    exit 1
+  fi
+
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that the new implementation only checks if CHANGELOG.md was modified but doesn't validate the content format, which is a significant regression from the original validation logic that was removed.

Medium

@dbarrosop dbarrosop closed this Aug 15, 2025
@dbarrosop dbarrosop reopened this Aug 15, 2025
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 886ff0b

@dbarrosop dbarrosop merged commit 71b6c89 into main Aug 15, 2025
4 checks passed
@dbarrosop dbarrosop deleted the changelog branch August 15, 2025 12:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant