Skip to content

Conversation

VolodymyrBg
Copy link

@VolodymyrBg VolodymyrBg commented Sep 8, 2025

  • Ensure short_git_version handles both git describe outputs:
    • tagged form ends with g
    • non-tagged form returns without g
  • We now strip an optional leading g and consistently take the first 7 characters.
  • This prevents dropping the first hex digit when the hash lacks g and aligns with the rest of the codebase, which uses 7-char hashes without the g prefix.

Summary by CodeRabbit

  • New Features

    • None.
  • Bug Fixes

    • Standardized the displayed Git commit abbreviation to a consistent 7 characters, removing any leading “g” and improving readability across version screens, CLI outputs, and logs.
  • Refactor

    • Simplified the logic used to derive the short Git version, reducing edge cases and ensuring consistent behavior across builds and tags without altering public interfaces.

Copy link

coderabbitai bot commented Sep 8, 2025

Walkthrough

Refactors short_git_version in crates/libzkp/src/utils.rs to consistently derive a 7-character commit abbreviation by splitting GIT_VERSION on hyphens, removing a leading 'g' if present, and truncating to seven characters. No public API signatures were changed.

Changes

Cohort / File(s) Change summary
Version string utils
crates/libzkp/src/utils.rs
Simplified short_git_version: split GIT_VERSION by hyphen, take last segment, strip leading 'g', return first 7 chars. Previously conditionally sliced based on length. No signature changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A nibble of git, a dash of delight,
Seven neat crumbs from a commit’s bite.
I hop through hyphens, past the pesky “g”,
Trim to a tidy tag—so sleek, so free.
With whiskers twitching, I stamp the build: ✓
Short, sweet, certain—carrot-fully distilled. 🥕

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
crates/libzkp/src/utils.rs (1)

18-20: Edge case: exact tag builds can return the tag name, not a hash

At an exact tag, git describe --always returns the tag itself (e.g., v1.2.3), so split('-').next_back() yields the tag; the function would emit "v1.2.3" (truncated) instead of a 7-char hex. If callers expect a hex abbrev, release builds at tags will be inconsistent.

Two low-friction fixes (either is fine):

  • Make git describe always include the -g<hash> suffix by adding --long.
  • Or scan from the end for the first segment that looks like g[0-9a-f]+ or [0-9a-f]+ before truncating.

Apply this minimal change if you prefer the --long approach:

@@
-const GIT_VERSION: &str = git_version!(args = ["--abbrev=7", "--always"]);
+const GIT_VERSION: &str = git_version!(args = ["--abbrev=7", "--always", "--long"]);

Or, keep current args and harden the parsing:

@@ pub(crate) fn short_git_version() -> String {
-    let last = GIT_VERSION.split('-').next_back().unwrap();
-    let hash = last.strip_prefix('g').unwrap_or(last);
-    hash.chars().take(7).collect()
+    let cand = GIT_VERSION
+        .rsplit('-')
+        .find(|s| s.starts_with('g') || s.chars().all(|c| c.is_ascii_hexdigit()))
+        .unwrap_or(GIT_VERSION);
+    let hash = cand.strip_prefix('g').unwrap_or(cand);
+    hash.chars().take(7).collect()

Optional test suggestion (to guard both forms and exact-tag): extract the parsing into a small helper fn short_from(s: &str) -> String and unit-test inputs like v1.2.3-45-gabcdef1, abcdef1, and v1.2.3.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a5a7844 and b8bd5fd.

📒 Files selected for processing (1)
  • crates/libzkp/src/utils.rs (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/libzkp/src/utils.rs (1)
crates/libzkp/src/tasks/chunk.rs (1)
  • hash (190-190)
🔇 Additional comments (1)
crates/libzkp/src/utils.rs (1)

18-20: LGTM: simpler and correct normalization of g-prefixed vs bare hashes

The new logic robustly strips an optional leading 'g' and safely truncates to 7 chars without indexing panics. Matches the PR goal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant