Sync common files from infra repository

.bootc-dev-infra-commit.txt

@@ -1 +1 @@
-55772cd1ed6efa2f315f6a1cb03b80c575037932
+0c34c5b394926053b2fbe1106320f37bba500afa

AGENTS.md

@@ -57,7 +57,10 @@ the commit message text.
 ## Code guidelines
 
 The [REVIEW.md](REVIEW.md) file describes expectations around
-testing, code quality, commit messages, commit organization, etc. If you're
+testing, code quality, commit messages, commit organization, etc.
+Language-specific guidelines are in
+[REVIEW_RUST.md](REVIEW_RUST.md) and
+[REVIEW_GOLANG.md](REVIEW_GOLANG.md). If you're
 creating a change, it is strongly encouraged after each 
 commit and especially when the agent thinks a task is complete
 to spawn a subagent to perform a review using guidelines (alongside

REVIEW.md

@@ -26,9 +26,10 @@ easy to generate a *lot* of code for unit tests unnecessarily).
 ### Separating Parsing from I/O
 
 A recurring theme is structuring code for testability. Split parsers from data
-reading: have the parser accept a `&str`, then have a separate function that
-reads from disk and calls the parser. This makes unit testing straightforward
-without filesystem dependencies.
+reading: have the parser accept the raw data (e.g. a string), then have a
+separate function that reads from disk and calls the parser. This makes unit
+testing straightforward without filesystem dependencies. See the
+language-specific review guides for concrete examples.
 
 ### Test Assertions
 
@@ -48,9 +49,7 @@ or `sed`.
 
 Try to avoid having shell script longer than 50 lines. This commonly occurs
 in build system and tests. For the build system, usually there's higher
-level ways to structure things (Justfile e.g.) and several of our projects
-use the `cargo xtask` pattern to put arbitrary "glue" code in Rust using
-the `xshell` crate to keep it easy to run external commands.
+level ways to structure things (Justfile e.g.).
 
 ### Constants and Magic Values
 
@@ -64,10 +63,10 @@ value was chosen.
 
 ### Don't ignore (swallow) errors
 
-Avoid the `if let Ok(v) = ... { }` in Rust, or `foo 2>/dev/null || true`
-pattern in shell script by default. Most errors should be propagated by
-default. If not, it's usually appropriate to at least log error messages
-at a `tracing::debug!` or equivalent level.
+Avoid swallowing errors (e.g. `foo 2>/dev/null || true` in shell script).
+Most errors should be propagated by default. If not, it's usually appropriate
+to at least log error messages at a debug level. See the language-specific
+review guides for concrete anti-patterns.
 
 Handle edge cases explicitly: missing data, malformed input, offline systems.
 Error messages should provide clear context for diagnosis.
@@ -192,25 +191,28 @@ functionality, ensure equivalent coverage exists.
 
 When multiple contributors co-author a PR, bring in an independent reviewer.
 
-## Rust-Specific Guidance
+## Dependencies
 
-Prefer rustix over `libc`. All `unsafe` code must be very carefully
-justified.
+New dependencies should be justified. Consider alternatives: "I'm curious if
+you did any comparative analysis at all with alternatives?"
 
-### Dependencies
+Prefer well-maintained libraries with active communities. Glance at existing
+reverse dependencies to gauge adoption (e.g. on crates.io for Rust, or
+pkg.go.dev for Go). Consider project-level dependency policies (e.g.
+`cargo deny` for Rust).
 
-New dependencies should be justified. Glance at existing reverse dependencies
-on crates.io to see if a crate is widely used. Consider alternatives: "I'm
-curious if you did any comparative analysis at all with alternatives?"
-
-Prefer well-maintained crates with active communities. Consider `cargo deny`
-policies when adding dependencies.
-
-### API Design
+## API Design
 
 When adding new commands or options, think about machine-readable output early.
 JSON is generally preferred for that.
 
 Keep helper functions in appropriate modules. Move command output formatting
 close to the CLI layer, keeping core logic functions focused on their primary
 purpose.
+
+## Language-Specific Guidance
+
+The following guides cover language-specific review expectations:
+
+- [REVIEW_RUST.md](REVIEW_RUST.md) — Rust projects
+- [REVIEW_GOLANG.md](REVIEW_GOLANG.md) — Go projects

REVIEW_GOLANG.md

@@ -0,0 +1,130 @@
+# Go-Specific Review Guidelines
+
+These guidelines supplement the general [REVIEW.md](REVIEW.md) with
+Go-specific expectations.
+
+## Separating Parsing from I/O
+
+Have the parser accept a string or `io.Reader`, then have a separate function
+that opens the file and calls the parser:
+
+```go
+// ✅ Good: parser is a pure function, easy to unit test
+func parseConfig(data string) (*Config, error) { ... }
+
+func loadConfig(path string) (*Config, error) {
+    data, err := os.ReadFile(path)
+    if err != nil {
+        return nil, err
+    }
+    return parseConfig(string(data))
+}
+```
+
+## Don't Ignore (Swallow) Errors
+
+Avoid discarding errors with the blank identifier. Most errors should be
+returned to the caller. If not, at least log the error:
+
+```go
+// ❌ Avoid: error is silently swallowed
+_ = doSomething()
+
+// ✅ Good: propagate
+if err := doSomething(); err != nil {
+    return err
+}
+
+// ✅ OK if the error is truly ignorable: log it
+if err := doSomething(); err != nil {
+    log.Debug("ignoring error", "err", err)
+}
+```
+
+## Gomega and Test Assertions
+
+We use [gomega](https://github.com/onsi/gomega) for test assertions. Follow
+these conventions:
+
+### Use `g.Eventually` for Polling
+
+Gomega's `Eventually` handles polling, timeouts, and failure reporting.
+
+### Return `(T, error)` from `Eventually` Callbacks
+
+Return the specific field you care about and let gomega matchers describe the
+expectation declaratively. This produces better failure messages because
+gomega can show what the value actually was vs. what was expected.
+
+```go
+// ✅ Good: return the field, match with gomega
+g.Eventually(func() ([]metav1.Condition, error) {
+    var p bootcv1alpha1.BootcNodePool
+    err := k8sClient.Get(ctx, client.ObjectKey{Name: name}, &p)
+    return p.Status.Conditions, err
+}).Should(ContainElement(And(
+    HaveField("Type", bootcv1alpha1.PoolDegraded),
+    HaveField("Status", metav1.ConditionTrue),
+    HaveField("Reason", bootcv1alpha1.PoolNodeDegraded),
+)))
+
+// ❌ Avoid: assertions inside the callback with Succeed()
+g.Eventually(func(g Gomega) {
+    var p bootcv1alpha1.BootcNodePool
+    g.Expect(k8sClient.Get(ctx, ...)).To(Succeed())
+    cond := apimeta.FindStatusCondition(p.Status.Conditions, ...)
+    g.Expect(cond).NotTo(BeNil())
+    g.Expect(cond.Status).To(Equal(...))
+}).Should(Succeed())
+```
+
+### Return the Narrowest Type
+
+Extract exactly the field you want to assert on — labels, conditions,
+ownerReference — rather than returning the whole object or a `bool`:
+
+```go
+// Labels
+g.Eventually(func() (map[string]string, error) {
+    var n corev1.Node
+    err := k8sClient.Get(ctx, client.ObjectKey{Name: name}, &n)
+    return n.Labels, err
+}).Should(HaveKey(bootcv1alpha1.LabelManaged))
+
+// OwnerReference
+g.Eventually(func() (*metav1.OwnerReference, error) {
+    var bn bootcv1alpha1.BootcNode
+    err := k8sClient.Get(ctx, client.ObjectKey{Name: name}, &bn)
+    return metav1.GetControllerOf(&bn), err
+}).Should(And(Not(BeNil()), HaveField("Name", pool.Name)))
+```
+
+### Use Composed Matchers for Struct Assertions
+
+Prefer `HaveField` and `ContainElement(And(...))` to match on struct fields
+declaratively rather than manually extracting fields and asserting one by one:
+
+```go
+// ✅ Good: declarative, one expression
+g.Expect(conditions).To(ContainElement(And(
+    HaveField("Type", bootcv1alpha1.PoolDegraded),
+    HaveField("Status", metav1.ConditionTrue),
+    HaveField("Reason", bootcv1alpha1.PoolInvalidSpec),
+)))
+
+// ❌ Avoid: manual lookup + sequential field assertions
+cond := apimeta.FindStatusCondition(conditions, bootcv1alpha1.PoolDegraded)
+g.Expect(cond).NotTo(BeNil())
+g.Expect(cond.Status).To(Equal(metav1.ConditionTrue))
+g.Expect(cond.Reason).To(Equal(bootcv1alpha1.PoolInvalidSpec))
+```
+
+### Assert Specific Errors When Expected
+
+When a test expects a particular error, match on the concrete error type or
+value:
+
+```go
+// ✅ Good: we know the API server should reject this
+g.Expect(err).To(MatchError(apierrors.IsInvalid, "IsInvalid"))
+```

REVIEW_RUST.md

@@ -0,0 +1,60 @@
+# Rust-Specific Review Guidelines
+
+These guidelines supplement the general [REVIEW.md](REVIEW.md) with
+Rust-specific expectations.
+
+## Separating Parsing from I/O
+
+Have the parser accept a `&str`, then have a separate function that reads from
+disk and calls the parser:
+
+```rust
+// ✅ Good: parser is a pure function, easy to unit test
+fn parse_config(data: &str) -> Result<Config> { ... }
+
+fn load_config(path: &Path) -> Result<Config> {
+    let data = std::fs::read_to_string(path)?;
+    parse_config(&data)
+}
+```
+
+## Don't Ignore (Swallow) Errors
+
+Avoid the `if let Ok(v) = ... { }` pattern which silently discards the error
+branch. Most errors should be propagated with `?`. If not, at least log the
+error:
+
+```rust
+// ❌ Avoid: error is silently swallowed
+if let Ok(v) = do_something() {
+    use_value(v);
+}
+
+// ✅ Good: propagate
+let v = do_something()?;
+
+// ✅ OK if the error is truly ignorable: log it
+match do_something() {
+    Ok(v) => use_value(v),
+    Err(e) => tracing::debug!("ignoring error: {e}"),
+}
+```
+
+## Shell Scripts
+
+Several of our projects use the `cargo xtask` pattern to put arbitrary "glue"
+code in Rust using the `xshell` crate to keep it easy to run external commands.
+This is preferred over long shell scripts.
+
+## Code Formatting
+
+After making changes to any .rs files, `cargo fmt` should be run.
+There are CI jobs that lint and expect `cargo fmt` to be a no-op.
+Failing to properly format rust code will result in wasted CI
+resources.
+
+## General
+
+Prefer rustix over `libc`. All `unsafe` code must be very carefully
+justified.
+