Skip to content
Merged
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
111 changes: 76 additions & 35 deletions src/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ impl Environment {
Path::new("./k8s/environments")
.join(name)
.to_str()
.unwrap()
.ok_or("Invalid path for environment name")?
.to_string(),
);
Comment on lines 90 to 95

Copilot AI Mar 30, 2026

Copy link

Choose a reason for hiding this comment

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

to_str() returns Option<&str>, and using ok_or("Invalid path for environment name")? yields a Result<_, &str>. In this function the error type is Box<dyn std::error::Error>, and &str does not implement Error, so this ? will not type-check. Convert the failure into a concrete error type (e.g., std::io::Error::other(...)) or map it into a boxed error explicitly before using ?.

Copilot uses AI. Check for mistakes.

Expand All @@ -114,7 +114,7 @@ impl Environment {
Path::new("./k8s/environments")
.join(&self.name)
.to_str()
.unwrap()
.ok_or("Invalid path for environment name")?
.to_string(),
);

Expand Down Expand Up @@ -235,7 +235,7 @@ impl Serialize for Service {
{
service.insert("version".to_string(), "latest".to_string());
} else if self.major_version.is_some() {
let mut version_str = self.major_version.unwrap().to_string();
let mut version_str = self.major_version.unwrap_or(0).to_string();

if let Some(minor) = self.minor_version {
version_str.push_str(&format!(".{}", minor));
Expand Down Expand Up @@ -267,15 +267,19 @@ impl<'de> Deserialize<'de> for Service {
}
};

let name = service.get("name").expect("Service name is required");
let name = service
.get("name")
.ok_or_else(|| serde::de::Error::missing_field("name"))?;
let namespace = service
.get("namespace")
.unwrap_or(&"default".to_string())
.to_string();

let path = service.get("path").map(|s| s.as_str()).unwrap_or("");
let build = service.get("build");
let version = service.get("version").expect("Service version is required");
let version = service
.get("version")
.ok_or_else(|| serde::de::Error::missing_field("version"))?;

let mut major_version: Option<i32> = None;
let mut minor_version: Option<i32> = None;
Expand Down Expand Up @@ -370,43 +374,38 @@ impl Service {
&& self.minor_version.is_none()
&& self.patch_version.is_none()
{
return self.tag.as_ref().unwrap().to_string();
return self.tag.clone().unwrap_or_else(|| "latest".to_string());
}

let mut version_str = self.major_version.unwrap_or(0).to_string();

if let Some(minor) = self.minor_version {
version_str.push_str(&format!(".{}", minor));
}

if let Some(patch) = self.patch_version {
version_str.push_str(&format!(".{}", patch));
}

if let Some(tag) = &self.tag {
format!(
"{}.{}.{}-{}",
self.major_version.unwrap_or(0),
self.minor_version.unwrap_or(0),
self.patch_version.unwrap_or(0),
tag
)
} else {
// Handle cases where patch_version is None
if self.patch_version.is_none() {
format!(
"{}.{}",
self.major_version.unwrap_or(0),
self.minor_version.unwrap_or(0),
)
} else {
format!(
"{}.{}.{}",
self.major_version.unwrap_or(0),
self.minor_version.unwrap_or(0),
self.patch_version.unwrap_or(0)
)
}
version_str.push_str(&format!("-{}", tag));
}

version_str
}

pub fn get_version_without_tag(&self) -> String {
format!(
"{}.{}.{}",
self.major_version.unwrap_or(0), // Changed from unwrap()
self.minor_version.unwrap_or(0), // Changed from unwrap()
self.patch_version.unwrap_or(0) // Changed from unwrap()
)
let mut version_str = self.major_version.unwrap_or(0).to_string();

if let Some(minor) = self.minor_version {
version_str.push_str(&format!(".{}", minor));
}

if let Some(patch) = self.patch_version {
version_str.push_str(&format!(".{}", patch));
}
Comment on lines +380 to +406

Copilot AI Mar 30, 2026

Copy link

Choose a reason for hiding this comment

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

get_version() / get_version_without_tag() now omit the minor segment when minor_version is None, even if patch_version is Some(_). Because bump_patch_version() can set patch_version while leaving minor_version as None, this can produce ambiguous versions like "1.1" (is that minor=1 or patch=1?) instead of a stable "1.0.1"-style output. Consider enforcing the invariant that patch_version implies minor_version.is_some() (e.g., set minor to 0 when bumping/formatting), or adjust the formatting to include a 0 minor segment whenever a patch segment exists.

Copilot uses AI. Check for mistakes.

version_str
}

pub fn get_path(&self) -> String {
Expand Down Expand Up @@ -667,4 +666,46 @@ mod tests {
}
);
}

#[test]
fn test_regression_get_version() {
let s1 = Service {
name: "srv1".to_string(),
namespace: "default".to_string(),
path: None,
build: None,
major_version: Some(2601),
minor_version: None,
patch_version: None,
tag: Some("rc".to_string()),
};
assert_eq!(s1.get_version(), "2601-rc");
assert_eq!(s1.get_version_without_tag(), "2601");

let s2 = Service {
name: "srv2".to_string(),
namespace: "default".to_string(),
path: None,
build: None,
major_version: Some(2601),
minor_version: None,
patch_version: None,
tag: Some("rc.0.0".to_string()),
};
assert_eq!(s2.get_version(), "2601-rc.0.0");
assert_eq!(s2.get_version_without_tag(), "2601");

let s3 = Service {
name: "srv3".to_string(),
namespace: "default".to_string(),
path: None,
build: None,
major_version: Some(2601),
minor_version: Some(0),
patch_version: None,
tag: Some("rc".to_string()),
};
assert_eq!(s3.get_version(), "2601.0-rc");
assert_eq!(s3.get_version_without_tag(), "2601.0");
}
}
Loading