Fix missing Path import lost during merge of #89#100
Merged
Conversation
- Remove pre-fix bug description block from test_validate_files_respects_owned_globs_with_excluded_extensions; replace with a concise description of what the test verifies - Correct the simplified owned_globs pattern in the comment to match the actual fixture config - Replace "THIS ASSERTION WILL FAIL ON MAIN" prose with a one-line summary - Update matches_globs doc comment to describe the function rather than its callsites - Replace brittle project_builder.rs:172 line-number reference with a description of intent - Add missing `use std::path::Path` import (compile error introduced in #89)
The bare `cargo check` job only checked the lib/bin crates and was strictly less thorough than the existing Test Suite and Lints jobs, both of which compile all targets. The redundant job also introduced a stale-cache risk: if its artifacts were reused from a prior run, compile errors in source files could go undetected. Removing it so that Test Suite (cargo test) and Lints (cargo clippy --all-targets) are the authoritative compile checks.
dduugg
changed the title
Uses Swatinem/rust-cache@v2 in the Check, Test Suite, and Lints jobs. Setting cache-on-failure: false (the default) ensures that a failed build never writes a stale cache entry, which is how a missing import in #89 was able to pass CI despite being a compile error. Restores the cargo check job that was removed in the prior commit, since it gives a clearer signal for compile errors than a mixed test/lint failure.
dduugg
changed the title
dduugg
changed the title
martinemde
approved these changes
Mar 6, 2026
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
2 participants
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
Fixes a missing
use std::path::Pathimport insrc/runner.rsthat wasintroduced during the merge of #89, and cleans up stale comments left in
the tests from that PR.
Root cause of the missing import
The PR branch had
use std::{path::Path, process::Command};. The basebranch (
main) had the imports on separate lines:use std::path::PathBuf;/use std::process::Command;. When GitHubmerged the PR, it split the grouped import but silently dropped
Path,keeping only
PathBufandprocess::Command.Pre-merge CI passed (the PR branch was correct). The post-merge CI run
on main caught the failure, but by then the merge had already landed.
The right long-term fix is enabling "Require branches to be up to date
before merging" in branch protection, which re-runs CI against the
post-merge state before the merge button becomes available.
Stale comments cleaned up in tests
"THIS ASSERTION WILL FAIL ON MAIN" from
test_validate_files_respects_owned_globs_with_excluded_extensions;replaced with a concise description of what the test verifies.
"**/*.{rb,tsx,erb}"→"{gems,config,javascript,ruby,components}/**/*.{rb,tsx,erb}".matches_globsdoc comment to describe the function.// Apply the same filtering logic as project_builder.rs:172with a description of intent.