fix: Sort operator versions by semver version (#336)

* fix: Sort operator versions by semver version instead of alphabetically * changelog * Improve error message * Fix pre-commit stuff * Add node to pre-commit CI action * Improve error reporting in web/build.rs * Install node thingies in pre-commit action * changelog * changelog * Apply suggestions from code review Co-authored-by: Techassi <[email protected]> * cargo fmt --------- Co-authored-by: Techassi <[email protected]>
stackabletech · Nov 20, 2024 · 0f62acd · 0f62acd
1 parent a60b677
commit 0f62acd
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 19 deletions.
diff --git a/.github/workflows/pr_pre-commit.yml b/.github/workflows/pr_pre-commit.yml
@@ -19,6 +19,11 @@ jobs:
       - uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 #v30
         with:
           github_access_token: ${{ secrets.GITHUB_TOKEN }}
+      - uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2
+        with:
+          node-version: 18
+          cache: yarn
+      - run: yarn install --frozen-lockfile
       - uses: stackabletech/actions/run-pre-commit@9bd13255f286e4b7a654617268abe1b2f37c3e0a # v0.3.0
         with:
           rust: ${{ env.RUST_TOOLCHAIN_VERSION }}

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -23,7 +23,7 @@ repos:
       - id: fmt
         args: ["--all", "--", "--check"]
       - id: clippy
-        args: ["--all-targets", "--", "-D", "warnings"]
+        args: ["--all-targets", "--all-features", "--", "-D", "warnings"]
 
   - repo: https://github.com/adrienverge/yamllint
     rev: 81e9f98ffd059efe8aa9c1b1a42e5cce61b640c6 # 1.35.1

diff --git a/README.md b/README.md
@@ -65,7 +65,7 @@ hooks are:
 - [`yamllint`](https://github.com/adrienverge/yamllint): Runs linting on all YAML files
 - [`markdownlint`](https://github.com/igorshubovych/markdownlint-cli): Runs linting on all Markdown files
 - [`prettier`](https://github.com/pre-commit/mirrors-prettier): Runs prettier on files located in `web`
-- `cargo clippy -- -D warnings`: Runs Clippy on all files and errors on warnings
+- `cargo clippy --all-targets --all-features -- -D warnings`: Runs Clippy on all files and errors on warnings
 - `cargo fmt -- --check`: Checks if Rust code needs formatting
 - `cargo xtask gen-comp`: Runs shell completions generation for `stackablectl`
 - `cargo xtask gen-man`: Runs man page generation for `stackablectl`
@@ -77,4 +77,3 @@ hooks are:
 [pre-commit]: https://pre-commit.com/
 [web-readme]: ./web/README.md
 [lib-readme]: ./rust/stackable-cockpit/README.md
-[xtasks]: ./xtask/src/main.rs
diff --git a/crate-hashes.json b/crate-hashes.json
diff --git a/rust/stackablectl/CHANGELOG.md b/rust/stackablectl/CHANGELOG.md
@@ -8,6 +8,11 @@ All notable changes to this project will be documented in this file.
 
 - Add shell completions for Nushell and Elvish ([#337]).
 
+### Fixed
+
+- Sort operator versions by semver version instead of alphabetically ([#336]).
+
+[#336]: https://github.com/stackabletech/stackable-cockpit/pull/336
 [#337]: https://github.com/stackabletech/stackable-cockpit/pull/337
 
 ## [24.11.0] - 2024-11-18

diff --git a/rust/stackablectl/src/cmds/operator.rs b/rust/stackablectl/src/cmds/operator.rs
@@ -130,6 +130,12 @@ pub enum CmdError {
     #[snafu(display("invalid repository name"))]
     InvalidRepoName { source: InvalidRepoNameError },
 
+    #[snafu(display("invalid semantic helm chart version {version:?}"))]
+    InvalidHelmChartVersion {
+        source: semver::Error,
+        version: String,
+    },
+
     #[snafu(display("unknown repository name '{name}'"))]
     UnknownRepoName { name: String },
 
@@ -524,16 +530,15 @@ where
         Some(entries) => {
             let mut versions = entries
                 .iter()
-                .map(|e| Version::parse(&e.version))
-                .map_while(|r| match r {
-                    Ok(v) => Some(v),
-                    Err(_) => None,
+                .map(|entry| {
+                    Version::parse(&entry.version).with_context(|_| InvalidHelmChartVersionSnafu {
+                        version: entry.version.clone(),
+                    })
                 })
-                .map(|v| v.to_string())
-                .collect::<Vec<String>>();
-
+                .collect::<Result<Vec<_>, _>>()?;
             versions.sort();
-            Ok(versions)
+
+            Ok(versions.iter().map(|version| version.to_string()).collect())
         }
         None => Ok(vec![]),
     }

diff --git a/web/build.rs b/web/build.rs
@@ -16,18 +16,22 @@ fn main() {
     ] {
         println!("cargo:rerun-if-changed={tracked_file}");
     }
-    let vite_status = Command::new("yarn")
+
+    let mut vite_command = Command::new("yarn");
+    vite_command
         .arg("run")
         .arg("build")
         .arg("--outDir")
         .arg(&vite_out_dir)
         .arg("--base")
-        .arg("/ui/")
-        .status()
-        .unwrap();
-    if !vite_status.success() {
-        panic!("web-ui build failed: {vite_status}");
-    }
+        .arg("/ui/");
+
+    let vite_status = vite_command.status();
+    match vite_status {
+        Ok(vite_status) if vite_status.success() => {}
+        _ => panic!("web-ui build failed: command {vite_command:?} resulted in {vite_status:?}"),
+    };
+
     let mut asset_map = phf_codegen::Map::new();
     for asset_file in vite_out_dir.join("assets").read_dir().unwrap() {
         let asset_file = asset_file.unwrap();