Skip to content

Commit 9baa44e

Browse files
committed
Fix regression in package.yml metadata.owner key
As far as I can tell, this key was not intentionally removed and its presence is missed. Based on issues reported, We're restoring support rather than fix all the other things that depend on it. Related to: rubyatscale/code_ownership#141
1 parent c3cace6 commit 9baa44e

File tree

2 files changed

+52
-1
lines changed

2 files changed

+52
-1
lines changed

src/project.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ pub mod deserializers {
111111
#[derive(Deserialize)]
112112
pub struct RubyPackage {
113113
pub owner: Option<String>,
114+
pub metadata: Option<Metadata>,
114115
}
115116

116117
#[derive(Deserialize)]

src/project_builder.rs

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,17 @@ fn ruby_package_owner(path: &Path) -> Result<Option<String>, Error> {
309309
let file = File::open(path).change_context(Error::Io)?;
310310
let deserializer: deserializers::RubyPackage = serde_yaml::from_reader(file).change_context(Error::SerdeYaml)?;
311311

312-
Ok(deserializer.owner)
312+
let top_level_owner = deserializer.owner;
313+
let metadata_owner = deserializer.metadata.and_then(|metadata| metadata.owner);
314+
315+
// Error if both are present to avoid ambiguity
316+
match (top_level_owner.as_ref(), metadata_owner.as_ref()) {
317+
(Some(_), Some(_)) => Err(error_stack::report!(Error::Io).attach_printable(format!(
318+
"Package at {} has both 'owner' and 'metadata.owner' defined. Please use only one.",
319+
path.display()
320+
))),
321+
_ => Ok(top_level_owner.or(metadata_owner)),
322+
}
313323
}
314324

315325
fn javascript_package_owner(path: &Path) -> Result<Option<String>, Error> {
@@ -334,4 +344,44 @@ mod tests {
334344
fn test_glob_match() {
335345
assert!(glob_match(OWNED_GLOB, "script/.eslintrc.js"));
336346
}
347+
348+
#[test]
349+
fn test_ruby_package_owner_top_level() {
350+
let yaml = "owner: TeamA\n";
351+
let temp_file = tempfile::NamedTempFile::new().unwrap();
352+
std::fs::write(temp_file.path(), yaml).unwrap();
353+
354+
let owner = ruby_package_owner(temp_file.path()).unwrap();
355+
assert_eq!(owner, Some("TeamA".to_string()));
356+
}
357+
358+
#[test]
359+
fn test_ruby_package_owner_metadata() {
360+
let yaml = "metadata:\n owner: TeamB\n";
361+
let temp_file = tempfile::NamedTempFile::new().unwrap();
362+
std::fs::write(temp_file.path(), yaml).unwrap();
363+
364+
let owner = ruby_package_owner(temp_file.path()).unwrap();
365+
assert_eq!(owner, Some("TeamB".to_string()));
366+
}
367+
368+
#[test]
369+
fn test_ruby_package_owner_errors_when_both_present() {
370+
let yaml = "owner: TeamA\nmetadata:\n owner: TeamB\n";
371+
let temp_file = tempfile::NamedTempFile::new().unwrap();
372+
std::fs::write(temp_file.path(), yaml).unwrap();
373+
374+
let result = ruby_package_owner(temp_file.path());
375+
assert!(result.is_err());
376+
}
377+
378+
#[test]
379+
fn test_ruby_package_owner_no_owner() {
380+
let yaml = "name: my_package\n";
381+
let temp_file = tempfile::NamedTempFile::new().unwrap();
382+
std::fs::write(temp_file.path(), yaml).unwrap();
383+
384+
let owner = ruby_package_owner(temp_file.path()).unwrap();
385+
assert_eq!(owner, None);
386+
}
337387
}

0 commit comments

Comments
 (0)