ci: enforce buildifier format + lint on every PR#11
Merged
Conversation
Enables a Buildifier workflow that runs //.github/workflows:buildifier.check on every PR and main push. The check runs in diff+warn mode, so the gate fires on both formatting drift and lint findings. Changes required to bring the tree to a clean state: - MODULE.bazel: bump buildifier_prebuilt 6.1.2 -> 8.5.1.2. The 6.1.2 wrapper hardcodes `external/<repo>/file/buildifier` for its runfiles lookup, but Bazel 9's bzlmod runfiles put the binary at the runfiles root, so `bazel run` silently failed with no output. - playwright/repositories.bzl: add explicit `return None` at the end of _playwright_repo_impl and _define_browsers_impl so all execution paths return a value (fixes return-value lint). - playwright/private/known_browsers.bzl: add module docstring. - playwright/private/integrity_map.bzl: silence the print() lint on the deprecation warning we deliberately emit for the misspelled `playright_repo_name` attr. - playwright/private/browser_targets.bzl: flesh out docstrings on compute_browser_targets and render_workspace_files with Args/Returns sections, plus buildifier's auto-format pass. - tools/config_setting/BUILD.bazel: drop a stray blank line. - .github/workflows/buildifier.yaml.disabled -> buildifier.yml: rename to enable the workflow, switch `bazel` -> `bazelisk` for consistency with tests.yml, and drop the now-default --enable_bzlmod flag. Note: playwright/private/browser_targets.bzl:25-28 has a duplicate `debian13` condition that looks like a typo introduced by the debian13 support commit. Worth a separate fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Enables a
BuildifierCI check that runs on every PR and main push, gating both format (mode=diff) and lint (lint=warn) findings on all Starlark files.The wiring already existed in
.github/workflows/BUILD.bazeland.github/workflows/buildifier.yaml.disabled— this PR brings the tree to a clean state and turns the workflow on.Why the dep bump
buildifier_prebuilt6.1.2's generated wrapper looks for the binary atexternal/<repo>/file/buildifier, but Bazel 9's bzlmod runfiles put it at the runfiles root. The result:bazelisk run //.github/workflows:buildifier.checksilently exits 1 with no output (xargs invokes an empty path). Bumping to 8.5.1.2 fixes the wrapper.Lint findings fixed
playwright/repositories.bzl:28,87—_playwright_repo_impland_define_browsers_implhad implicit returns after a conditionalreturn ctx.repo_metadata(...). Added explicitreturn None.playwright/private/known_browsers.bzl:1— file had no module docstring. Added one.playwright/private/integrity_map.bzl:122—print()is buildifier-flagged, but it's the deliberate deprecation warning for the legacyplayright_repo_nameattr. Added# buildifier: disable=printso the warning behavior is preserved without the lint failure.playwright/private/browser_targets.bzl:66,113—compute_browser_targetsandrender_workspace_fileshad one-liner docstrings missingArgs:andReturns:sections. Expanded.Format fixes
Buildifier auto-applied minor blank-line tweaks to
browser_targets.bzlandintegrity_map.bzl.Workflow tweaks
.github/workflows/buildifier.yaml.disabled→buildifier.ymlbazel→bazeliskfor consistency withtests.yml(and to honor.bazelversion)--enable_bzlmodflag (default in Bazel 9)branches: [main]filter onpull_requestto matchtests.ymlbehaviorNoted but NOT fixed in this PR
playwright/private/browser_targets.bzl:25-28has a duplicatedebian13condition that looks like a typo from the debian13 platform-support commit — buildifier doesn't flag duplicate branches, but it's worth fixing separately.Test plan
Build and Testjob remains greenbazelisk run //.github/workflows:buildifier.checkexits 0🤖 Generated with Claude Code