Skip to content

build: follow up vars edge cases#31

Merged
tisonkun merged 1 commit into
mainfrom
follow-up-gix-for-build
May 8, 2026
Merged

build: follow up vars edge cases#31
tisonkun merged 1 commit into
mainfrom
follow-up-gix-for-build

Conversation

@tisonkun
Copy link
Copy Markdown
Contributor

@tisonkun tisonkun commented May 7, 2026

This follows up #30

Signed-off-by: tison <wander4096@gmail.com>
Comment thread scopeql/src/version.rs
Comment on lines +36 to +41
let commit_short = if commit.len() >= 8 {
let (commit_short, _) = commit.split_at(8);
commit_short
} else {
commit
};
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

When build with cargo install scopeql, there is no Git repo and thus commit = "". split_at(8) would panic.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request centralizes the initialization of build environment variables, corrects a typo in a Cargo build instruction, and adds a safety check for Git commit hash truncation to prevent panics. Review feedback recommends explicitly defining Cargo watch paths to ensure build metadata remains accurate and suggests using a more idiomatic string slicing method for the commit hash.

Comment thread scopeql/build.rs
Comment thread scopeql/src/version.rs
@tisonkun tisonkun merged commit 90acb52 into main May 8, 2026
12 checks passed
@tisonkun tisonkun deleted the follow-up-gix-for-build branch May 8, 2026 00:09
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