Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ codeowners for-team Payroll
#### Generate but do not stage the file

```sh
codeowners generate --skip-stage
codeownership validate --skip-stage
```

#### Run without using the cache
Expand Down
6 changes: 3 additions & 3 deletions src/ownership/codeowners_file_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ pub fn parse_for_team(team_name: String, codeowners_file: &str) -> Result<Vec<Te
let mut output = vec![];
let mut current_section: Option<TeamOwnership> = None;
let input: String = codeowners_file.replace(&FileGenerator::disclaimer().join("\n"), "");
let error_message = "CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file";
let error_message = "CODEOWNERS out of date. Run `codeownership validate` to update the CODEOWNERS file";
Copy link
Contributor

Choose a reason for hiding this comment

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

codeowners generate is correct for codeowners-rs, but not for the ruby gem code_ownership that wraps it.

We'll likely need to at least update code_ownership to fix this bug and possibly rethink how errors are handled in the ruby to rust FFI.

Copy link
Author

Choose a reason for hiding this comment

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

Sounds good. How would I be able to update this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Possibilities:

  • Make the error message generic "CODEOWNERS out of date. Regenerate the CODEOWNERS file to fix."
  • Update RunResult to include error codes, which could be used by code_ownership to customize error messages
  • Temporary solution: the gem post-processes the output: gsub codeowners generate with bin/codeownership validate
  • ?


for line in input.trim_start().lines() {
match line {
Expand Down Expand Up @@ -345,7 +345,7 @@ mod tests {
let team_ownership = parse_for_team("@Foo".to_string(), codeownership_file);
assert!(
team_ownership
.is_err_and(|e| e.to_string() == "CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file")
.is_err_and(|e| e.to_string() == "CODEOWNERS out of date. Run `codeownership validate` to update the CODEOWNERS file")
);
Ok(())
}
Expand All @@ -360,7 +360,7 @@ mod tests {
let team_ownership = parse_for_team("@Foo".to_string(), codeownership_file);
assert!(
team_ownership
.is_err_and(|e| e.to_string() == "CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file")
.is_err_and(|e| e.to_string() == "CODEOWNERS out of date. Run `codeownership validate` to update the CODEOWNERS file")
);
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion src/ownership/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 `bin/{} validate` to update the CODEOWNERS file", executable_name)
}
Error::InvalidTeam { name: _, path: _ } => "Found invalid team annotations".to_owned(),
}
Expand Down
4 changes: 2 additions & 2 deletions tests/executable_name_config_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ fn test_validate_with_default_executable_name() -> Result<(), Box<dyn Error>> {
&["validate"],
false,
OutputStream::Stdout,
predicate::str::contains("Run `codeowners generate`"),
predicate::str::contains("Run `codeownership validate`"),
)?;
Ok(())
}
Expand Down Expand Up @@ -59,7 +59,7 @@ fn test_default_executable_name_full_error_message() -> Result<(), Box<dyn Error
OutputStream::Stdout,
predicate::eq(indoc! {"

CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file
CODEOWNERS out of date. Run `codeownership validate` to update the CODEOWNERS file

"}),
)?;
Expand Down
2 changes: 1 addition & 1 deletion tests/invalid_project_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ fn test_validate() -> Result<(), Box<dyn Error>> {
OutputStream::Stdout,
predicate::eq(indoc! {"

CODEOWNERS out of date. Run `codeowners generate` to update the CODEOWNERS file
CODEOWNERS out of date. Run `codeownership validate` to update the CODEOWNERS file

Code ownership should only be defined for each file in one way. The following files have declared ownership in multiple ways

Expand Down
2 changes: 1 addition & 1 deletion tests/run_config_executable_override_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ fn test_run_config_executable_path_overrides_default() -> Result<(), Box<dyn Err
error_msg
);
assert!(
!error_msg.contains("codeowners generate"),
!error_msg.contains("codeownership validate"),
"Error should not contain default when overridden"
);

Expand Down