Skip to content
Merged
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
4 changes: 4 additions & 0 deletions .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ env:
jobs:
release:
name: Release - ${{ matrix.platform.os_name }}
env:
COMAN_RELEASE_VERSION: ${{ github.event.release.tag_name }}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The environment variable COMAN_RELEASE_VERSION is set but may not be used elsewhere in the workflow.

#ai-review-inline

strategy:
matrix:
platform:
Expand Down Expand Up @@ -53,6 +55,8 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v5
- name: Strip version leading v
run: echo "COMAN_RELEASE_VERSION=${COMAN_RELEASE_VERSION##v}" >> $GITHUB_ENV
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The command to strip 'v' prefix might fail if tag_name is empty or malformed.

#ai-review-inline

- name: Install toolchain
uses: dtolnay/rust-toolchain@stable
with:
Expand Down
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion coman/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
[package]
name = "coman"
version = "0.8.9"
edition = "2024"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Version field was removed, ensure this is intentional and that versioning is handled elsewhere.

#ai-review-inline

description = "Compute Manager for managing HPC compute"
authors = ["Ralf Grubenmann <ralf.grubenmann@sdsc.ethz.ch>"]
Expand Down
4 changes: 4 additions & 0 deletions coman/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ use anyhow::Result;
use vergen_gix::{BuildBuilder, CargoBuilder, Emitter, GixBuilder};

fn main() -> Result<()> {
if let Ok(val) = std::env::var("COMAN_RELEASE_VERSION") {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider adding error handling for the environment variable parsing to avoid unexpected behavior if the variable is not a valid string.

Suggested change
if let Ok(val) = std::env::var("COMAN_RELEASE_VERSION") {
if let Ok(val) = std::env::var("COMAN_RELEASE_VERSION") {
println!("cargo:rustc-env=CARGO_PKG_VERSION={}", val);
} else {
eprintln!("Warning: COMAN_RELEASE_VERSION not set or invalid");
}

#ai-review-inline

println!("cargo:rustc-env=CARGO_PKG_VERSION={}", val);
}
println!("cargo:rerun-if-env-changed=COMAN_RELEASE_VERSION");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The rerun-if-env-changed instruction should be placed before any conditional logic to ensure proper dependency tracking.

Suggested change
println!("cargo:rerun-if-env-changed=COMAN_RELEASE_VERSION");
println!("cargo:rerun-if-env-changed=COMAN_RELEASE_VERSION");
if let Ok(val) = std::env::var("COMAN_RELEASE_VERSION") {
println!("cargo:rustc-env=CARGO_PKG_VERSION={}", val);
}

#ai-review-inline

let build = BuildBuilder::all_build()?;
let gix = GixBuilder::all_git()?;
let cargo = CargoBuilder::all_cargo()?;
Expand Down
9 changes: 1 addition & 8 deletions coman/src/cli/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,14 +485,7 @@ pub enum CscsSystemCommands {

pub const COMAN_VERSION: &str = env!("CARGO_PKG_VERSION");

const VERSION_MESSAGE: &str = concat!(
env!("CARGO_PKG_VERSION"),
"-",
env!("VERGEN_GIT_DESCRIBE"),
" (",
env!("VERGEN_BUILD_DATE"),
")"
);
const VERSION_MESSAGE: &str = concat!(env!("CARGO_PKG_VERSION"), " (", env!("VERGEN_BUILD_DATE"), ")");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The VERSION_MESSAGE now omits the git describe information which might reduce debuggability.

Suggested change
const VERSION_MESSAGE: &str = concat!(env!("CARGO_PKG_VERSION"), " (", env!("VERGEN_BUILD_DATE"), ")");
Consider re-adding the git describe info for better version tracking in production builds.

#ai-review-inline


pub fn version() -> String {
// let current_exe_path = PathBuf::from(clap::crate_name!()).display().to_string();
Expand Down