Merge pull request #819 from XeroAPI/conventional-commit-breaking-check

Switch to using conventional commit for breaking change flag detection

Author: the-chris-mitchell

.github/workflows/api-diff.yml

@@ -12,32 +12,32 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  test-branch-logic:
+  test-conventional-commit-logic:
     runs-on: ubuntu-latest
     permissions:
       contents: read
     steps:
     - name: Checkout code
       uses: actions/checkout@v4
-    
-    - name: Run branch logic unit tests
+
+    - name: Run conventional commit logic unit tests
       run: ./scripts/api-diff/api-diff.test.sh
-  
+
   api-diff:
     runs-on: ubuntu-latest
-    needs: test-branch-logic
+    needs: test-conventional-commit-logic
     permissions:
       contents: read
       pull-requests: write
-    
+
     steps:
     - name: Checkout code
       uses: actions/checkout@v4
       with:
         fetch-depth: 0
-    
+
     - name: Make script executable
       run: chmod +x scripts/api-diff/api-diff.sh
-    
+
     - name: Run API diff check
-      run: ./scripts/api-diff/api-diff.sh
+      run: ./scripts/api-diff/api-diff.sh

README.md

@@ -2,7 +2,7 @@
 An OpenAPI (Swagger) specification for the Xero API with OAuth 2.0 security schema.
 
 ## Description
-This repository holds the official Xero [OpenAPI](https://www.openapis.org/) descriptions.   
+This repository holds the official Xero [OpenAPI](https://www.openapis.org/) descriptions.
 
 OpenAPI spec 3.0
 In Release (used for one or more SDKs)
@@ -19,43 +19,33 @@ In Release (used for one or more SDKs)
 
 ## Contribution guide
 
-[Conventional commit](https://www.conventionalcommits.org/en/v1.0.0/#summary) format should be used when contributing to this repo.
+Use [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/#summary) for all commits.
 
-**Context:**
+Why this matters:
 
-Release notes are generated using [semantic-release](https://github.com/semantic-release/semantic-release). In brief this package analyses all the commits in the repo to determine the next version number, generate the release notes and publish the package, hence the commit message is important for the release to happen successfully. Please read more on the conventional commit [documentation](https://www.conventionalcommits.org/en/v1.0.0/#summary) before landing your commits, each commit message will determine the release notes.
+- We use [semantic-release](https://github.com/semantic-release/semantic-release) to calculate versions automatically.
+- Consistent commit formatting keeps versioning predictable and release notes accurate.
 
-**Example:**
+Versioning rules:
 
-If your commit message is - ``feat: added a new parameter in the get_account method`` - It will do a **minor version update**. 
+- `feat:` -> minor version bump
+- `fix:` -> patch version bump
+- `!` in the header or `BREAKING CHANGE:` footer -> major version bump
 
-Following release notes will be generated -
+Examples:
 
+```text
+feat(accounting): add includeArchived query parameter to GET /Accounts
 ```
-Release notes 
 
- Feature - 
-   - added a new parameter in the get_account method 
+```text
+fix(files): correct nullable schema for File.ContentLength
 ```
 
-If your commit message is - ``fix: fixed the null issue with get_invoice method`` - It will do a **patch version update**. 
-
-Following release notes will be generated - 
-
-```
-Release notes 
-
- Fix - 
-   - fixed the null issue with get_invoice method
-```
-
-
-if your commit message is as below it will do a **major version update** (Breaking change)
+```text
+feat(payroll-nz)!: remove deprecated EarningsRateID field from Payslip response
 
-```
-chore!: drop support for Node 6
-
-BREAKING CHANGE: use JavaScript features not available in Node 6
+BREAKING CHANGE: clients must use EarningsRateIdentifier instead of EarningsRateID.
 ```
 
 
@@ -64,7 +54,7 @@ We are using [OpenAPI generator](https://github.com/OpenAPITools/openapi-generat
 
 ## Preview
 ### Online
-There are lots of tools available for viewing and editing OpenAPI descriptions in a nicely formatted way. A popular tool is SwaggerHub - a version of which is [hosted here](https://app.swaggerhub.com/home). 
+There are lots of tools available for viewing and editing OpenAPI descriptions in a nicely formatted way. A popular tool is SwaggerHub - a version of which is [hosted here](https://app.swaggerhub.com/home).
 
 Once you sign up or login, you can create a new API under your account and import a Xero API spec.
 
@@ -99,10 +89,13 @@ This repository includes automated API diff checking using [oasdiff](https://git
 ./scripts/api-diff/api-diff.sh xero_accounting.yaml
 ```
 
-### Branch Naming Convention
-Branches containing `breaking` anywhere in the name will allow breaking changes without failing the build. All other branches will fail if breaking changes are detected.
+### Breaking Change Enforcement
+By default, API diff checks fail when breaking changes are detected.
+
+Breaking changes are allowed only when commit messages include a Conventional Commits breaking marker:
 
-**Examples:** `breaking-api-v2`, `feature-breaking-change`, `api-breaking-update`
+- `!` in the commit header, for example `feat!: remove deprecated endpoint`
+- `BREAKING CHANGE:` in the commit body/footer
 
 ### Full Documentation
 For detailed usage, configuration options, environment variables, and integration details, see [scripts/api-diff/README.md](scripts/api-diff/README.md).

scripts/api-diff/README.md

@@ -27,37 +27,39 @@ Main script that compares OpenAPI specifications against the master branch.
 - `BASE_BRANCH` - Branch to compare against (default: `origin/master`)
 
 ### `api-diff.test.sh`
-Unit tests for the branch logic pattern matching used in GitHub Actions.
+Unit tests for conventional commit breaking marker detection used in GitHub Actions.
 
 **Usage:**
 ```bash
 ./scripts/api-diff/api-diff.test.sh
 ```
 
 Tests validate that:
-- Branches containing `breaking` anywhere in the name are correctly identified
-- Other branches are handled with breaking change enforcement
+- Commits with `!` in the conventional commit header are correctly identified
+- Commits with `BREAKING CHANGE:` footer are correctly identified
+- Other commits are handled with breaking change enforcement
 
 ## Integration
 
 These scripts are integrated into the GitHub Actions workflow at `.github/workflows/api-diff.yml`:
-- **test-branch-logic** job - Runs unit tests
+- **test-conventional-commit-logic** job - Runs unit tests
 - **api-diff** job - Runs API diff checks with conditional breaking change enforcement
 
-### Branch Naming Convention
-The GitHub Actions workflow automatically adjusts its behavior based on branch names:
+### Conventional Commit Breaking Markers
+The API diff script automatically adjusts behavior based on commit messages:
 
 **Allow Breaking Changes:**
-- Any branch containing `breaking` in the name
-- Examples: `breaking-api-v2`, `feature-breaking-change`, `api-breaking-update`
+- Commit header with `!`, for example: `feat!: remove deprecated endpoint`
+- Commit header with scope and `!`, for example: `feat(api)!: remove deprecated endpoint`
+- Commit body/footer containing `BREAKING CHANGE: ...`
 - The `--fail-on-breaking` flag is NOT passed to the script
 
 **Fail on Breaking Changes:**
-- All other branches (main, master, develop, feature branches, etc.)
+- Commits without these conventional commit breaking markers
 - The `--fail-on-breaking` flag IS passed to the script
 - Build will fail if breaking changes are detected
 
-This allows developers to explicitly signal when they're working on breaking changes by including `breaking` in their branch name.
+This keeps enforcement aligned with [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/#summary) and semantic-release expectations.
 
 ## Known Limitations
 

scripts/api-diff/api-diff.sh

@@ -26,6 +26,40 @@ FAIL_ON_BREAKING=false
 TARGET_FILE=""
 DRY_RUN=false
 
+detect_breaking_commit_marker() {
+    local commits="$1"
+
+    # Conventional Commits breaking indicators:
+    # 1) An exclamation mark in the type/scope header, e.g. feat!: ... or feat(api)!: ...
+    # 2) A BREAKING CHANGE footer in the commit body
+    if echo "$commits" | grep -Eiq '^[[:space:]]*[a-z]+(\([^)]+\))?!:'; then
+        return 0
+    fi
+
+    if echo "$commits" | grep -Eiq 'BREAKING[ -]CHANGE:'; then
+        return 0
+    fi
+
+    return 1
+}
+
+get_commit_messages() {
+    # For tests and local overrides
+    if [ -n "$COMMIT_MESSAGES" ]; then
+        echo "$COMMIT_MESSAGES"
+        return 0
+    fi
+
+    # In GitHub Actions PRs, scan all commit subjects + bodies in the PR range.
+    if [ -n "$GITHUB_BASE_REF" ]; then
+        git log --format='%s%n%b%n----' "origin/$GITHUB_BASE_REF..HEAD" 2>/dev/null || true
+        return 0
+    fi
+
+    # Local default: inspect HEAD commit only.
+    git log -1 --format='%s%n%b' 2>/dev/null || true
+}
+
 # Parse arguments
 for arg in "$@"; do
     if [ "$arg" = "--fail-on-breaking" ]; then
@@ -37,26 +71,14 @@ for arg in "$@"; do
     fi
 done
 
-# Detect current branch: GitHub Actions PR branch, or local git branch
-# Allow CURRENT_BRANCH env var to override for testing
-if [ -n "$CURRENT_BRANCH" ]; then
-    # Use explicitly set CURRENT_BRANCH (for testing)
-    :
-elif [ -n "$GITHUB_HEAD_REF" ]; then
-    # Use GitHub Actions PR branch
-    CURRENT_BRANCH="$GITHUB_HEAD_REF"
-else
-    # Use local git branch
-    CURRENT_BRANCH=$(git branch --show-current 2>/dev/null || echo "")
-fi
-
-# If --fail-on-breaking not explicitly set, determine based on branch name
+# If --fail-on-breaking not explicitly set, determine based on conventional commit markers.
 if [ "$FAIL_ON_BREAKING" = false ]; then
-    if [[ "$CURRENT_BRANCH" == *breaking* ]]; then
-        echo "Branch '$CURRENT_BRANCH' contains 'breaking', allowing breaking changes"
+    COMMIT_TEXT=$(get_commit_messages)
+    if detect_breaking_commit_marker "$COMMIT_TEXT"; then
+        echo "Detected conventional commit breaking marker ('!' in header or 'BREAKING CHANGE:' footer), allowing breaking changes"
         FAIL_ON_BREAKING=false
     else
-        echo "Branch '$CURRENT_BRANCH' does not contain 'breaking', failing on breaking changes"
+        echo "No conventional commit breaking marker found, failing on breaking changes"
         FAIL_ON_BREAKING=true
     fi
 fi
@@ -67,7 +89,7 @@ if [ "$DRY_RUN" = true ]; then
     else
         echo "Mode: Allowing breaking changes"
     fi
-    echo "Dry run mode, exiting after branch check"
+    echo "Dry run mode, exiting after commit message check"
     exit 0
 fi
 
@@ -119,56 +141,56 @@ for file in $files; do
     TOTAL_FILES=$((TOTAL_FILES + 1))
     echo ""
     echo "========== $file =========="
-    
+
     # Get the file from master branch
     if ! git show "$BASE_BRANCH:$file" > "$TEMP_DIR/$file" 2>/dev/null; then
         echo "ℹ️  New file (does not exist in master branch)"
         continue
     fi
-    
+
     # Verify the temp file was created
     if [ ! -f "$TEMP_DIR/$file" ]; then
         echo "❌ Failed to create temp file"
         continue
     fi
-    
+
     # Note: oasdiff has some non-deterministic behavior in change counts due to
     # unordered map iteration in Go. Error counts are consistent, but warning
     # counts may vary by ~2-3% between runs. This is a known limitation.
-    
+
     # Run oasdiff changelog
     echo "--- Changelog ---"
     set +e
     CHANGELOG_OUTPUT=$(docker run --rm -v "$(pwd)":/current -v "$TEMP_DIR":/base "$DOCKER_IMAGE" changelog --include-path-params /base/"$file" /current/"$file" 2>&1)
     CHANGELOG_EXIT=$?
     set -e
-    
+
     echo "$CHANGELOG_OUTPUT"
-    
+
     if [ $CHANGELOG_EXIT -eq 0 ]; then
         echo "✓ Changelog generated successfully"
     else
         echo "⚠ Could not generate changelog (exit code: $CHANGELOG_EXIT)"
     fi
-    
+
     # Run breaking changes check
     echo ""
     echo "--- Breaking changes check ---"
     set +e
     BREAKING_OUTPUT=$(docker run --rm -v "$(pwd)":/current -v "$TEMP_DIR":/base "$DOCKER_IMAGE" breaking --fail-on WARN --include-path-params /base/"$file" /current/"$file" 2>&1)
     BREAKING_EXIT=$?
     set -e
-    
+
     echo "$BREAKING_OUTPUT"
-    
+
     if [ $BREAKING_EXIT -eq 0 ]; then
         echo "✓ No breaking changes detected"
     else
         echo "⚠ Breaking changes detected (exit code: $BREAKING_EXIT)"
         BREAKING_CHANGES_FOUND=true
         FILES_WITH_BREAKING_CHANGES+=("$file")
     fi
-    
+
     PROCESSED_FILES=$((PROCESSED_FILES + 1))
 done
 
@@ -189,7 +211,7 @@ if [ "$BREAKING_CHANGES_FOUND" = true ]; then
             echo "::warning file=${file}::Breaking changes detected in this API spec file"
         fi
     done
-    
+
     if [ "$FAIL_ON_BREAKING" = true ]; then
         echo ""
         echo "Exiting with error due to breaking changes"