diff --git a/.github/workflows/api-diff.yml b/.github/workflows/api-diff.yml index 8ceb59842..3ba967cc4 100644 --- a/.github/workflows/api-diff.yml +++ b/.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 \ No newline at end of file + run: ./scripts/api-diff/api-diff.sh diff --git a/README.md b/README.md index 7bbaf477c..0f4acdc64 100644 --- a/README.md +++ b/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). diff --git a/scripts/api-diff/README.md b/scripts/api-diff/README.md index 374ed418f..75d9e6f47 100644 --- a/scripts/api-diff/README.md +++ b/scripts/api-diff/README.md @@ -27,7 +27,7 @@ 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 @@ -35,29 +35,31 @@ Unit tests for the branch logic pattern matching used in GitHub Actions. ``` 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 diff --git a/scripts/api-diff/api-diff.sh b/scripts/api-diff/api-diff.sh index 30af3b1af..b5c09823c 100755 --- a/scripts/api-diff/api-diff.sh +++ b/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,38 +141,38 @@ 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 ---" @@ -158,9 +180,9 @@ for file in $files; do 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 @@ -168,7 +190,7 @@ for file in $files; do 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" diff --git a/scripts/api-diff/api-diff.test.sh b/scripts/api-diff/api-diff.test.sh index edd9cdbc8..bce73e8a9 100755 --- a/scripts/api-diff/api-diff.test.sh +++ b/scripts/api-diff/api-diff.test.sh @@ -1,11 +1,11 @@ #!/bin/bash -# Unit test for api-diff.sh branch logic -# Tests the script's branch detection and FAIL_ON_BREAKING setting +# Unit test for api-diff.sh conventional commit logic +# Tests commit message detection and FAIL_ON_BREAKING setting set -e -echo "=== Unit Test: api-diff.sh Branch Logic ===" +echo "=== Unit Test: api-diff.sh Conventional Commit Logic ===" echo TESTS_PASSED=0 @@ -13,20 +13,19 @@ TESTS_FAILED=0 SCRIPT_PATH="scripts/api-diff/api-diff.sh" -# Helper function to test script with branch -test_branch() { - local branch_name="$1" - local expected_mode="$2" # "allow" or "fail" +# Helper function to test script with commit messages +test_commit_messages() { + local commit_messages="$1" + local expected_mode="$2" # "allow-breaking-changes" or "block-breaking-changes" local test_name="$3" - - echo "Testing: $test_name (branch: $branch_name)" - - # Run the script in dry-run mode with CURRENT_BRANCH set + + echo "Testing: $test_name (commit: $commit_messages)" + + # Run the script in dry-run mode with COMMIT_MESSAGES set local output - output=$(CURRENT_BRANCH="$branch_name" "$SCRIPT_PATH" --dry-run 2>&1) - - # Check the output for the expected message - if [[ "$expected_mode" == "allow" ]]; then + output=$(COMMIT_MESSAGES="$commit_messages" "$SCRIPT_PATH" --dry-run 2>&1) + + if [[ "$expected_mode" == "allow-breaking-changes" ]]; then if echo "$output" | grep -q "Mode: Allowing breaking changes"; then echo " ✓ PASS: Correctly allows breaking changes" TESTS_PASSED=$((TESTS_PASSED + 1)) @@ -35,7 +34,7 @@ test_branch() { echo "$output" TESTS_FAILED=$((TESTS_FAILED + 1)) fi - elif [[ "$expected_mode" == "fail" ]]; then + elif [[ "$expected_mode" == "block-breaking-changes" ]]; then if echo "$output" | grep -q "Mode: Failing on breaking changes"; then echo " ✓ PASS: Correctly fails on breaking changes" TESTS_PASSED=$((TESTS_PASSED + 1)) @@ -48,31 +47,27 @@ test_branch() { echo } -# Test cases: test_branch "branch-name" "expected-mode" "description" -# expected-mode: "allow" = allows breaking changes, "fail" = fails on breaking +# Test cases: test_commit_messages "commit_messages" "expected_mode" "test_name" +# expected_mode: "allow-breaking-changes" = allows breaking changes, "block-breaking-changes" = fails on breaking -echo "--- Branches that SHOULD allow breaking changes ---" -test_branch "breaking-api-changes" "allow" "Branch with 'breaking' at start" -test_branch "feature-breaking-change" "allow" "Branch with 'breaking' in middle" -test_branch "fix-breaking-bug" "allow" "Branch with 'breaking' in middle" -test_branch "api-breaking-changes" "allow" "Branch with 'breaking' in middle" -test_branch "update-breaking-endpoint" "allow" "Branch with 'breaking' in middle" +echo "--- Commit messages that SHOULD allow breaking changes ---" +test_commit_messages "feat!: remove deprecated endpoint" "allow-breaking-changes" "Header with ! marker" +test_commit_messages "feat(api)!: remove deprecated endpoint" "allow-breaking-changes" "Header with scope and ! marker" +test_commit_messages "feat: refactor API\n\nBREAKING CHANGE: response schema updated" "allow-breaking-changes" "BREAKING CHANGE footer" +test_commit_messages "fix: patch bug\n\nSome details\nBREAKING CHANGE: removed old field" "allow-breaking-changes" "BREAKING CHANGE footer after body" -echo "--- Branches that SHOULD fail on breaking changes ---" -test_branch "feature-new-endpoint" "fail" "Normal feature branch" -test_branch "main" "fail" "Main branch" -test_branch "master" "fail" "Master branch" -test_branch "develop" "fail" "Develop branch" -test_branch "add-openapi-diff-tool" "fail" "Current branch name" -test_branch "fix-api-bug" "fail" "Bug fix branch" -test_branch "feature-v2" "fail" "Version feature branch" +echo "--- Commit messages that SHOULD fail on breaking changes ---" +test_commit_messages "feat: add optional field" "block-breaking-changes" "Normal feature commit" +test_commit_messages "fix: resolve null issue" "block-breaking-changes" "Normal fix commit" +test_commit_messages "chore: update docs" "block-breaking-changes" "Chore commit" +test_commit_messages "docs: mention breaking behaviour in description" "block-breaking-changes" "Contains word breaking but no marker" echo "--- Test override with --fail-on-breaking ---" -# Test that --fail-on-breaking overrides branch logic -echo "Testing override: breaking branch with --fail-on-breaking" -output=$(CURRENT_BRANCH="breaking-test" "$SCRIPT_PATH" --dry-run --fail-on-breaking 2>&1) +# Test that --fail-on-breaking overrides commit message logic +echo "Testing override: commit with breaking marker and --fail-on-breaking" +output=$(COMMIT_MESSAGES="feat!: breaking api update" "$SCRIPT_PATH" --dry-run --fail-on-breaking 2>&1) if echo "$output" | grep -q "Mode: Failing on breaking changes"; then - echo " ✓ PASS: --fail-on-breaking overrides branch logic" + echo " ✓ PASS: --fail-on-breaking overrides commit logic" TESTS_PASSED=$((TESTS_PASSED + 1)) else echo " ✗ FAIL: --fail-on-breaking did not override, output:"