diff --git a/.github/workflows/sdk-update.yml b/.github/workflows/sdk-update.yml index 1c75f44..377a5e4 100644 --- a/.github/workflows/sdk-update.yml +++ b/.github/workflows/sdk-update.yml @@ -143,7 +143,14 @@ jobs: path.write_text(json.dumps(data, indent=2) + "\n") ' - # ----- Generate (each step continue-on-error so we can still PR) ------ + # ----- Generate + quality checks -------------------------------------- + # `make generate` is the only step the others truly depend on (they all + # need the regenerated tree). Format / lint / smoke / test all run + # independently after generate so reviewers see every signal on a single + # run instead of just "the first thing that broke." Each is + # continue-on-error so the workflow proceeds to open the bot PR + # regardless, and the "Determine generation status" step tiers the + # outcomes into one of three states. - name: make generate if: steps.fetch.outputs.changed == 'true' id: generate @@ -157,35 +164,48 @@ jobs: run: make format - name: make lint - if: steps.fetch.outputs.changed == 'true' && steps.format.outcome == 'success' + if: steps.fetch.outputs.changed == 'true' && steps.generate.outcome == 'success' id: lint continue-on-error: true run: make lint - name: make smoke-test - if: steps.fetch.outputs.changed == 'true' && steps.lint.outcome == 'success' + if: steps.fetch.outputs.changed == 'true' && steps.generate.outcome == 'success' id: smoke continue-on-error: true run: make smoke-test - name: make test - if: steps.fetch.outputs.changed == 'true' && steps.smoke.outcome == 'success' + if: steps.fetch.outputs.changed == 'true' && steps.generate.outcome == 'success' id: test continue-on-error: true run: make test + # Three-tier status so reviewers can tell apart the structural failure + # ("the SDK isn't buildable, don't merge") from the quality failure + # ("the SDK is buildable but has lint/test issues to look at"): + # + # generation-failed — `make generate` itself didn't succeed; the + # regenerated tree is missing or broken. PR opens + # with the spec update committed but no usable + # SDK; reviewer needs to investigate before merge. + # quality-issues — generate succeeded but one or more of + # format/lint/smoke/test failed. SDK is structurally + # valid; the issues are usually small and fixable. + # success — everything green; safe to merge once reviewed. - name: Determine generation status if: steps.fetch.outputs.changed == 'true' id: status run: | - if [ "${{ steps.generate.outcome }}" = "success" ] \ - && [ "${{ steps.format.outcome }}" = "success" ] \ - && [ "${{ steps.lint.outcome }}" = "success" ] \ - && [ "${{ steps.smoke.outcome }}" = "success" ] \ - && [ "${{ steps.test.outcome }}" = "success" ]; then + if [ "${{ steps.generate.outcome }}" != "success" ]; then + echo "result=generation-failed" >> "$GITHUB_OUTPUT" + elif [ "${{ steps.format.outcome }}" = "success" ] \ + && [ "${{ steps.lint.outcome }}" = "success" ] \ + && [ "${{ steps.smoke.outcome }}" = "success" ] \ + && [ "${{ steps.test.outcome }}" = "success" ]; then echo "result=success" >> "$GITHUB_OUTPUT" else - echo "result=failed" >> "$GITHUB_OUTPUT" + echo "result=quality-issues" >> "$GITHUB_OUTPUT" fi # ----- Skip-if-no-meaningful-changes ---------------------------------- @@ -283,7 +303,9 @@ jobs: TITLE="[auto] SDK update (${{ steps.classify.outputs.level }}) — spec ${{ steps.fetch.outputs.source_commit }}" # Build the exact label set for this run. `auto-generated` is - # always present; `breaking` and `generation-failed` are toggled. + # always present; `breaking`, `generation-failed`, and + # `quality-issues` are toggled based on this run's outcomes. + # See the "Determine generation status" step for tier definitions. ADD_LABELS=("auto-generated") REMOVE_LABELS=() if [ "${{ steps.analyze.outputs.has_breaking }}" = "true" ]; then @@ -291,11 +313,19 @@ jobs: else REMOVE_LABELS+=("breaking") fi - if [ "${{ steps.status.outputs.result }}" = "failed" ]; then - ADD_LABELS+=("generation-failed") - else - REMOVE_LABELS+=("generation-failed") - fi + case "${{ steps.status.outputs.result }}" in + generation-failed) + ADD_LABELS+=("generation-failed") + REMOVE_LABELS+=("quality-issues") + ;; + quality-issues) + ADD_LABELS+=("quality-issues") + REMOVE_LABELS+=("generation-failed") + ;; + success) + REMOVE_LABELS+=("generation-failed" "quality-issues") + ;; + esac ADD_LABELS_CSV="$(IFS=,; echo "${ADD_LABELS[*]}")" diff --git a/pyproject.toml b/pyproject.toml index d0e3039..49c0c53 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -105,27 +105,33 @@ known-first-party = ["bamboohr_sdk"] # the upstream templates and aren't worth fixing in our fork. # Hand-written code lives under bamboohr_sdk/client/ and stays under the # default rule set. +# +# RUF001/RUF002/RUF003 (ambiguous unicode in strings/docstrings/comments) are +# all ignored on generated paths — they fire on smart-quote characters (’, “”) +# that come straight from spec description text written by API designers. The +# characters aren't ambiguous in context; ruff is being pedantic about source +# we don't own. [tool.ruff.lint.per-file-ignores] "bamboohr_sdk/api/*.py" = [ - "B028", "B904", "F841", "N805", "RUF002", "RUF012", "RUF059", + "B028", "B904", "F841", "N805", "RUF001", "RUF002", "RUF003", "RUF012", "RUF059", "SIM101", "SIM105", "SIM108", "SIM118", "TC001", "TC003", "UP030", "UP031", ] "bamboohr_sdk/models/*.py" = [ - "F841", "N805", "N818", "RUF002", "RUF012", "SIM118", "TC001", "TC003", + "F841", "N805", "N818", "RUF001", "RUF002", "RUF003", "RUF012", "SIM118", "TC001", "TC003", ] "bamboohr_sdk/api_client.py" = [ - "B028", "B904", "F841", "RUF002", "RUF012", "RUF059", + "B028", "B904", "F841", "RUF001", "RUF002", "RUF003", "RUF012", "RUF059", "SIM101", "SIM105", "SIM108", "TC001", "TC003", "UP030", "UP031", ] "bamboohr_sdk/api_helper.py" = ["TC001", "TC003"] "bamboohr_sdk/api_response.py" = ["TC001", "TC003"] -"bamboohr_sdk/configuration.py" = ["B028", "B904", "RUF002", "UP030", "UP031"] +"bamboohr_sdk/configuration.py" = ["B028", "B904", "RUF001", "RUF002", "RUF003", "UP030", "UP031"] "bamboohr_sdk/exceptions.py" = ["N818", "SIM105"] -"bamboohr_sdk/rest.py" = ["B028", "B904", "RUF002", "SIM101", "SIM108"] +"bamboohr_sdk/rest.py" = ["B028", "B904", "RUF001", "RUF002", "RUF003", "SIM101", "SIM108"] # Hand-written client code: relax rules for type-only imports and # docstring-only stylistic noise. Real bugs would still surface via # mypy and the test suite. -"bamboohr_sdk/client/**/*.py" = ["B904", "RUF002", "RUF059", "TC001", "TC003"] +"bamboohr_sdk/client/**/*.py" = ["B904", "RUF001", "RUF002", "RUF003", "RUF059", "TC001", "TC003"] [tool.ruff.format] quote-style = "double" diff --git a/scripts/build_pr_body.py b/scripts/build_pr_body.py index d1fcd7c..d03cb36 100755 --- a/scripts/build_pr_body.py +++ b/scripts/build_pr_body.py @@ -17,7 +17,9 @@ --since-last-review Path to a pre-computed delta-vs-existing-branch text file. Pass --since-last-review="" (or omit) to render the "Initial PR" placeholder. - --generation-status One of: success, failed. Adds a banner if failed. + --generation-status One of: success, quality-issues, generation-failed. + Each tier shows a different banner (or none) at the + top of the PR body. --old-version Previous version (e.g. 1.0.0). --new-version Bumped version (e.g. 1.1.0); pass equal to old to skip the version-bump section. @@ -59,14 +61,24 @@ def parse_classification(text: str) -> tuple[str, str]: def build(args: argparse.Namespace) -> str: sections: list[str] = [] - # --- Banner for generation failures ------------------------------------ - if args.generation_status == "failed": + # --- Banner keyed off the three-tier generation status ----------------- + if args.generation_status == "generation-failed": + sections.append( + "> [!CAUTION]\n" + "> **`make generate` failed on this run.**\n" + "> The committed tree contains the spec update but no regenerated\n" + "> SDK — the package is not buildable as-is. Reproduce locally\n" + "> with `make generate` and resolve the underlying error before\n" + "> merging." + ) + elif args.generation_status == "quality-issues": sections.append( "> [!WARNING]\n" - "> **Generation, lint, or tests failed on this run.**\n" - "> The committed tree contains the spec update but may not contain\n" - "> a clean regenerated SDK. Reproduce locally with `make generate`\n" - "> then `make format && make lint && make test` before merging." + "> **Generation succeeded, but one or more quality checks failed.**\n" + "> The regenerated SDK is structurally valid; one or more of\n" + "> `make format` / `make lint` / `make smoke-test` / `make test`\n" + "> reported issues. See the workflow run for per-step details and\n" + "> reproduce locally with the failing target before merging." ) # --- Spec source metadata ---------------------------------------------- @@ -167,7 +179,7 @@ def parse_args() -> argparse.Namespace: parser.add_argument("--since-last-review", default="") parser.add_argument( "--generation-status", - choices=["success", "failed"], + choices=["success", "quality-issues", "generation-failed"], default="success", ) parser.add_argument("--old-version", default="")