Skip to content

Add a Bazel skill for this repository - #22600

Merged
redsun82 merged 5 commits into
mainfrom
redsun82-bazel-skill
Sep 17, 2026
Merged

redsun82 merged 5 commits into
mainfrom
redsun82-bazel-skill

Conversation

@redsun82

Copy link
Copy Markdown
Contributor

Agents (and humans) editing BUILD.bazel / *.bzl here have no cross-cutting reference: the only Bazel docs are per-area READMEs. This adds .github/skills/bazel/SKILL.md, establishing .github/skills/ in the process.

It is scoped to the standalone build, so it is useful to someone who only ever sees this repository. A counterpart exists for the internal module that also depends on this one; this is not a copy of it.

Traps-first, since that is the part not discoverable from any single file:

  • bazel build //... (and even bazel query //...) fails at the repo root, on the patched-module overlays under misc/bazel/registry, with an error naming an unrelated directory. Validate the target you changed instead.
  • There is deliberately no MODULE.bazel.lock.
  • linux_arm64 (kwarg) vs linux-arm64 (path string).
  • When extending the semmle_code stub is correct, and when it very much is not.

The rest covers where new code goes, the shared misc/bazel helpers, platform selection, the {CODEQL_PLATFORM} packaging placeholder, and adding dependencies.

Every claim is traceable to a file in this repo, and the load-bearing ones were verified by running them rather than by reading: standalone module resolution, the macro-generated target names, buildifier's load sorting, and the //... failure above.

Docs only. No Bazel files touched.

Agents (and humans) editing BUILD.bazel/*.bzl here had no cross-cutting
reference: the only Bazel docs are per-area READMEs. This collects the
conventions that are not discoverable from any single file — which
construct to reach for, the shared `misc/bazel` macros, the
`{CODEQL_PLATFORM}` packaging mechanism, checksum pinning, and what the
`semmle_code` stub does and does not cover.

A counterpart exists for the internal module that also depends on this
one. This is deliberately not a copy of it: it is scoped to the
standalone build, so it is useful to someone who only ever sees this
repository, and it says nothing that requires internal access.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 17, 2026 09:26
@redsun82
redsun82 requested a review from a team as a code owner September 17, 2026 09:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Four unresolved nit findings remain in the reviewed guidance.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite (auto)
Findings: 3 Low severity

Note

Copilot is running an experiment and ran this review at Lite.

Open (3)
What changed in this PR

Adds a repository-specific Bazel skill covering standalone builds, helpers, platforms, packaging, dependencies, and validation.

Changes:

  • Adds .github/skills/bazel/SKILL.md.
  • Documents Bazel conventions and troubleshooting guidance.
File Summary Final review comments
.github/​skills/​bazel/​SKILL.md New Bazel editing and validation guidance Four nit findings remain: clarify Rust wrapper scope (2 votes), qualify macro target and visibility examples (3 votes), broaden the @semmle_code stub condition (2 votes), and clarify _gen_binaries scope (1 vote).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/skills/bazel/SKILL.md Outdated
Comment thread .github/skills/bazel/SKILL.md Outdated
Comment thread .github/skills/bazel/SKILL.md Outdated
navntoft
navntoft previously approved these changes Sep 17, 2026

@navntoft navntoft left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

The wrapper is mandatory for binaries that go into a pack, not for every
rust_binary in the repo: swift-syntax-parse is a local debugging aid and
deliberately declares a plain one.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Each macro picks whether its helper targets are private, package default
or handed the caller's visibility. Keep the naming shapes as examples and
the grep-the-suffix tip, drop the blanket claim.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The documentation contains unresolved inaccuracies in target naming, visibility, platform fallback, stub requirements, and packaging override guidance.

Review effort: Lite (auto)
Findings: None

Note

Copilot is running an experiment and ran this review at Lite.

Resolved since last review (3)
Previously missed (1)

In code that hasn't changed since last review

Low severity Correct the generated-target naming guidance

.github/​skills/​bazel/​SKILL.md:61

The examples and visibility claim here are inaccurate. codeql_pack creates <name>-internal-all (misc/bazel/pkg.bzl:528-534), not <name>-all; codeql_rust_binary creates single_arch/<name> without setting visibility (misc/bazel/rust.bzl:11-20), so generated targets are not uniformly private. Please describe the macro-specific names and visibility instead of presenting one uniform scheme.

@redsun82
redsun82 requested a review from navntoft September 17, 2026 12:17
navntoft
navntoft previously approved these changes Sep 17, 2026
redsun82 and others added 2 commits September 17, 2026 15:14
No em dashes or contractions, matching the agent-facing docs already in
the repo. Also drops a few facts that would go stale without anything
catching them: the registry overlay glob, the platform count, and the
claim that nothing loads defs.bzl.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The documentation contains unresolved guidance inaccuracies, including one moderate issue.

Review effort: Lite (auto)
Findings: None

Note

Copilot is running an experiment and ran this review at Lite.

Previously missed (2)

In code that hasn't changed since last review

Low severity Document arch_overrides paths relative to the prefix

.github/​skills/​bazel/​SKILL.md:128

arch_overrides is compared with dest before pack_prefix is joined (misc/bazel/pkg.bzl:250-260), so an entry including tools/... or the pack prefix will not match. The current wording does not state this and can silently leave a file in the common zip; document that override entries omit the prefix.

Low severity Qualify the default installer and alias behavior

.github/​skills/​bazel/​SKILL.md:132

codeql_pack does not always create these targets: install_dest=None suppresses the installer and therefore the alias (misc/bazel/pkg.bzl:545-550), and installer_alias can also be disabled. Please say this is the default behavior so the documented target is not promised for packs that intentionally omit installation.

@redsun82
redsun82 merged commit 2304a1a into main Sep 17, 2026
7 checks passed
@redsun82
redsun82 deleted the redsun82-bazel-skill branch September 17, 2026 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants