Skip to content

Add generated archives for SHA-256 in gix-pack#2602

Merged
Sebastian Thiel (Byron) merged 2 commits into
GitoxideLabs:mainfrom
cruessler:run-gix-pack-tests-with-sha-256
May 27, 2026
Merged

Add generated archives for SHA-256 in gix-pack#2602
Sebastian Thiel (Byron) merged 2 commits into
GitoxideLabs:mainfrom
cruessler:run-gix-pack-tests-with-sha-256

Conversation

@cruessler
Copy link
Copy Markdown
Contributor

While working on this PR, I noticed that there’s a method, gix_pack::index::Version::hash(), that currently is only implemented for feature = "sha1". This method does not appear to be called, though (at least, my LSP found no references to it), so I was wondering whether it was redundant. Or, if it was not redundant, how it would need to be changed in order to support SHA-256.

This is part of #281.

@Byron Sebastian Thiel (Byron) force-pushed the run-gix-pack-tests-with-sha-256 branch from 36597c3 to 4f1bb83 Compare May 27, 2026 01:03
@Byron Sebastian Thiel (Byron) marked this pull request as ready for review May 27, 2026 01:03
Copilot AI review requested due to automatic review settings May 27, 2026 01:03
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

It's not useful either as there is no relationship between the Version
of the index file and the hash to use.
@Byron
Copy link
Copy Markdown
Member

That's a great catch!

I also don't know why this hash method is there and don't think it's useful in any way. Thus it's removed in my second commit.

@Byron Sebastian Thiel (Byron) merged commit 4f862a5 into GitoxideLabs:main May 27, 2026
32 checks passed
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.

3 participants