From 88f2b9be18fdfc3b408071b6c999a349ef47155c Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Fri, 6 Mar 2026 13:10:53 -0800 Subject: [PATCH 1/5] Remove redundant cargo check job from CI 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. --- .github/workflows/ci.yml | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e50da77..6f2aabc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,15 +18,6 @@ env: CARGO_TERM_COLOR: always jobs: - check: - name: Check - runs-on: ubuntu-latest - steps: - - name: Checkout sources - uses: actions/checkout@v4 - - - name: Run cargo check - run: cargo check test: name: Test Suite runs-on: ubuntu-latest @@ -58,7 +49,6 @@ jobs: needs: - test - lints - - check outputs: new_version: ${{ steps.check_for_version_changes.outputs.new_version }} changed: ${{ steps.check_for_version_changes.outputs.changed }} From b609895abd78e87a7d23a3e328289c73b34b03d8 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Fri, 6 Mar 2026 13:12:39 -0800 Subject: [PATCH 2/5] Add Rust build cache to CI and restore cargo check job 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. --- .github/workflows/ci.yml | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6f2aabc..e442821 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,6 +18,20 @@ env: CARGO_TERM_COLOR: always jobs: + check: + name: Check + runs-on: ubuntu-latest + steps: + - name: Checkout sources + uses: actions/checkout@v4 + + - name: Cache Rust build artifacts + uses: Swatinem/rust-cache@v2 + with: + cache-on-failure: false + + - name: Run cargo check + run: cargo check test: name: Test Suite runs-on: ubuntu-latest @@ -25,6 +39,11 @@ jobs: - name: Checkout sources uses: actions/checkout@v4 + - name: Cache Rust build artifacts + uses: Swatinem/rust-cache@v2 + with: + cache-on-failure: false + - name: Run cargo test with backtrace run: cargo test -- --nocapture env: @@ -38,6 +57,11 @@ jobs: - name: Checkout sources uses: actions/checkout@v4 + - name: Cache Rust build artifacts + uses: Swatinem/rust-cache@v2 + with: + cache-on-failure: false + - name: Run cargo fmt run: cargo fmt --all -- --check @@ -47,6 +71,7 @@ jobs: release: runs-on: macos-latest needs: + - check - test - lints outputs: From 2560f2a3ed2f5bd96cd3941b2fac21d4e35ed06a Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Fri, 6 Mar 2026 13:16:05 -0800 Subject: [PATCH 3/5] Revert "Add Rust build cache to CI and restore cargo check job" This reverts commit 2050667fd5bd955e2a5fa77f6afb9e424893bb09. --- .github/workflows/ci.yml | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e442821..6f2aabc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,20 +18,6 @@ env: CARGO_TERM_COLOR: always jobs: - check: - name: Check - runs-on: ubuntu-latest - steps: - - name: Checkout sources - uses: actions/checkout@v4 - - - name: Cache Rust build artifacts - uses: Swatinem/rust-cache@v2 - with: - cache-on-failure: false - - - name: Run cargo check - run: cargo check test: name: Test Suite runs-on: ubuntu-latest @@ -39,11 +25,6 @@ jobs: - name: Checkout sources uses: actions/checkout@v4 - - name: Cache Rust build artifacts - uses: Swatinem/rust-cache@v2 - with: - cache-on-failure: false - - name: Run cargo test with backtrace run: cargo test -- --nocapture env: @@ -57,11 +38,6 @@ jobs: - name: Checkout sources uses: actions/checkout@v4 - - name: Cache Rust build artifacts - uses: Swatinem/rust-cache@v2 - with: - cache-on-failure: false - - name: Run cargo fmt run: cargo fmt --all -- --check @@ -71,7 +47,6 @@ jobs: release: runs-on: macos-latest needs: - - check - test - lints outputs: From 65cb004e94a607cd315bbc082ba4b4af54e8feeb Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Fri, 6 Mar 2026 13:16:18 -0800 Subject: [PATCH 4/5] Revert "Remove redundant cargo check job from CI" This reverts commit d2679fa256fec5223bdd6b8a3002bc67c2727afe. --- .github/workflows/ci.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6f2aabc..e50da77 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,6 +18,15 @@ env: CARGO_TERM_COLOR: always jobs: + check: + name: Check + runs-on: ubuntu-latest + steps: + - name: Checkout sources + uses: actions/checkout@v4 + + - name: Run cargo check + run: cargo check test: name: Test Suite runs-on: ubuntu-latest @@ -49,6 +58,7 @@ jobs: needs: - test - lints + - check outputs: new_version: ${{ steps.check_for_version_changes.outputs.new_version }} changed: ${{ steps.check_for_version_changes.outputs.changed }} From f86e0afaef3cbc1539070d0ee734931bc153afc6 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Fri, 6 Mar 2026 15:57:58 -0800 Subject: [PATCH 5/5] Change executable_name to hold the full command string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, executable_name held just the binary name and the error message hardcoded " generate" as the subcommand: format!("CODEOWNERS out of date. Run `{} generate` ...", executable_name) This made it impossible for wrappers (like the code_ownership Ruby gem) to show the correct command in the error message, since their update command is not " generate". Now executable_name holds the full command to run. The format string no longer appends " generate". The default changes from "codeowners" to "codeowners generate" so existing behavior is preserved for users who do not customize the field. Users with a custom executable_name in config/code_ownership.yml will need to append their subcommand — e.g. change: executable_name: my-tool to: executable_name: my-tool generate --- src/config.rs | 6 ++--- src/ownership/validator.rs | 2 +- src/project.rs | 2 +- tests/executable_name_config_test.rs | 4 +-- .../config/code_ownership.yml | 2 +- tests/run_config_executable_override_test.rs | 27 +++++++++---------- 6 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/config.rs b/src/config.rs index 5282aa3..620185f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -68,7 +68,7 @@ fn vendored_gems_path() -> String { } fn default_executable_name() -> String { - "codeowners".to_string() + "codeowners generate".to_string() } fn default_ignore_dirs() -> Vec { @@ -134,7 +134,7 @@ mod tests { vec!["frontend/**/node_modules/**/*", "frontend/**/__generated__/**/*"] ); assert_eq!(config.vendored_gems_path, "vendored/"); - assert_eq!(config.executable_name, "codeowners"); + assert_eq!(config.executable_name, "codeowners generate"); Ok(()) } @@ -167,7 +167,7 @@ mod tests { fs::write(&config_path, config_str)?; let config_file = File::open(&config_path)?; let config: Config = serde_yaml::from_reader(config_file)?; - assert_eq!(config.executable_name, "codeowners"); + assert_eq!(config.executable_name, "codeowners generate"); Ok(()) } diff --git a/src/ownership/validator.rs b/src/ownership/validator.rs index 627c85c..16916c5 100644 --- a/src/ownership/validator.rs +++ b/src/ownership/validator.rs @@ -169,7 +169,7 @@ impl Error { Error::FileWithoutOwner { path: _ } => "Some files are missing ownership".to_owned(), Error::FileWithMultipleOwners { path: _, owners: _ } => "Code ownership should only be defined for each file in one way. The following files have declared ownership in multiple ways".to_owned(), Error::CodeownershipFileIsStale { executable_name } => { - format!("CODEOWNERS out of date. Run `{} generate` to update the CODEOWNERS file", executable_name) + format!("CODEOWNERS out of date. Run `{}` to update the CODEOWNERS file", executable_name) } Error::InvalidTeam { name: _, path: _ } => "Found invalid team annotations".to_owned(), } diff --git a/src/project.rs b/src/project.rs index 8293103..2d9cb71 100644 --- a/src/project.rs +++ b/src/project.rs @@ -221,7 +221,7 @@ mod tests { codeowners_file_path: PathBuf::from(".github/CODEOWNERS"), directory_codeowner_files: vec![], teams_by_name: HashMap::new(), - executable_name: "codeowners".to_string(), + executable_name: "codeowners generate".to_string(), }; let map = project.vendored_gem_by_name(); diff --git a/tests/executable_name_config_test.rs b/tests/executable_name_config_test.rs index 0b0eb6e..5832eb8 100644 --- a/tests/executable_name_config_test.rs +++ b/tests/executable_name_config_test.rs @@ -14,7 +14,7 @@ fn test_validate_with_custom_executable_name() -> Result<(), Box> { &["validate"], false, OutputStream::Stdout, - predicate::str::contains("Run `bin/codeownership generate`"), + predicate::str::contains("Run `bin/codeownership validate`"), )?; Ok(()) } @@ -42,7 +42,7 @@ fn test_custom_executable_name_full_error_message() -> Result<(), Box OutputStream::Stdout, predicate::eq(indoc! {" - CODEOWNERS out of date. Run `bin/codeownership generate` to update the CODEOWNERS file + CODEOWNERS out of date. Run `bin/codeownership validate` to update the CODEOWNERS file "}), )?; diff --git a/tests/fixtures/custom_executable_name/config/code_ownership.yml b/tests/fixtures/custom_executable_name/config/code_ownership.yml index fa25a7d..3980def 100644 --- a/tests/fixtures/custom_executable_name/config/code_ownership.yml +++ b/tests/fixtures/custom_executable_name/config/code_ownership.yml @@ -1,4 +1,4 @@ --- owned_globs: - "{app,config}/**/*.rb" -executable_name: "bin/codeownership" +executable_name: "bin/codeownership validate" diff --git a/tests/run_config_executable_override_test.rs b/tests/run_config_executable_override_test.rs index 35e1c4b..29074b5 100644 --- a/tests/run_config_executable_override_test.rs +++ b/tests/run_config_executable_override_test.rs @@ -13,21 +13,20 @@ fn test_run_config_executable_path_overrides_config_file() -> Result<(), Box Result<(), Box< let project_path = temp_dir.path(); git_add_all_files(project_path); - // This fixture has executable_name: "bin/codeownership" in config + // This fixture has executable_name: "bin/codeownership validate" in config let mut run_config = build_run_config(project_path, ".github/CODEOWNERS"); run_config.executable_name = None; // Explicitly no override let result = validate(&run_config, vec![]); - // Should use "bin/codeownership" from config file + // Should use "bin/codeownership validate" from config file assert!(!result.validation_errors.is_empty(), "Expected validation errors but got none"); let error_msg = result.validation_errors.join("\n"); assert!( - error_msg.contains("Run `bin/codeownership generate`"), - "Expected error to contain 'bin/codeownership generate' but got: {}", + error_msg.contains("Run `bin/codeownership validate`"), + "Expected error to contain 'bin/codeownership validate' but got: {}", error_msg ); @@ -75,14 +74,14 @@ fn test_run_config_executable_path_overrides_default() -> Result<(), Box