Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves CI diagnostics for azpysdk black by running Black in “check/diff” mode in CI and logging the diff output directly to the job logs, addressing the visibility gap described in #46417.
Changes:
- Run Black with
--check --diffwhenin_ci()is true (no in-place formatting during CI runs). - Capture and log Black stdout/stderr (diff + summary) when formatting issues are detected.
- Extend
format_directoryto accept acheck_onlyflag and adjust subprocess behavior accordingly.
| has_issues = result.returncode != 0 if check_only else "reformatted" in stderr_text | ||
|
|
||
| if has_issues: | ||
| if check_only: | ||
| logger.info( | ||
| f"The package {package_name} needs reformat. Run `azpysdk black .` locally from the package root to reformat." | ||
| f"The package {package_name} has black formatting issues. " | ||
| f"Run `azpysdk black .` locally from the package root to reformat." | ||
| ) |
There was a problem hiding this comment.
In CI (check_only=True), has_issues is based on any non-zero return code, but the log message always frames it as “black formatting issues”. Black uses different exit codes (e.g., 1 = would reformat, >1 = usage/internal error), so a black crash/invalid config would be misreported and its output logged at INFO. Consider branching on result.returncode in check-only mode (treat ==1 as formatting diff, and >1 as an execution failure logged at ERROR, ideally propagating the real return code).
closes #46417
CI example: https://dev.azure.com/azure-sdk/public/_build/results?buildId=6188730&view=logs&j=b70e5e73-bbb6-5567-0939-8415943fadb9&t=efe9ac6d-3fdb-50af-24e1-4319f36eecb4