-
Notifications
You must be signed in to change notification settings - Fork 211
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: wrong git version hash calculated if source dirty #4483
Conversation
@@ -66,7 +66,7 @@ fn set_code_version_inner() -> Result<(), String> { | |||
// state by replacing the middle with 0s. This should | |||
// be noticeable enough, while letting find out the | |||
// root commit anyway. | |||
format!("{}00000000{}", &hash[0..16], &hash[26..40]) | |||
format!("{}00000000{}", &hash[0..16], &hash[(40 - 16)..40]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe 16+8
instead of 40 - 16
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I agree this would be easier to reason about at a glance if it were
[0..16], [(16+8)..40]
or even
let zeros_count = 8;
format!(
"{}{}{}",
&hash[0..16],
"0".repeat(zeros_count),
&hash[(16 + zeros_count)..40]
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The damage is done, no need to overthink it. When it was as a constant (incorrect at at) no one worried; now that it isn't (and actually gives the correct value), there's 10 ways to express it so bikesheding starts. :D
Just hit the green button :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mathz are hard.
Hey, at least it's not off by 1.