Skip to content
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: sha256 not working #7099

Merged
merged 2 commits into from
Dec 6, 2023
Merged

fix: sha256 not working #7099

merged 2 commits into from
Dec 6, 2023

Conversation

hunjixin
Copy link
Contributor

@hunjixin hunjixin commented Dec 2, 2023

Closes #(Insert issue number closed by this PR)

Change Description

Background

Share context and relevant information for the PR: offline discussions, considerations, design decisions etc.

Bug Fix

If this PR is a bug fix, please let us know about:

  1. Problem - The reason for opening the bug
  2. Root cause - Discovered root cause after investigation
  3. Solution - How the bug was fixed

New Feature

If this PR introduces a new feature, describe it here.

Testing Details

How were the changes tested?

Breaking Change?

Does this change break any existing functionality? (API, CLI, Clients)

Additional info

Logs, outputs, screenshots of changes if applicable (CLI / GUI changes)

Contact Details

How can we get in touch with you if we need more info? (ex. [email protected])

@CLAassistant
Copy link

CLAassistant commented Dec 2, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@nopcoder nopcoder left a comment

Choose a reason for hiding this comment

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

LGTM, nitpicking suggestions.

Not in this PR, we can make the function and the implementation more strongly typed.

pkg/block/hashing_reader.go Show resolved Hide resolved
pkg/block/hashing_reader_test.go Outdated Show resolved Hide resolved
pkg/block/hashing_reader_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@nopcoder nopcoder left a comment

Choose a reason for hiding this comment

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

LGTM. thank you!

@nopcoder nopcoder enabled auto-merge (squash) December 3, 2023 08:51
@nopcoder nopcoder added include-changelog PR description should be included in next release changelog minor-change Used for PRs that don't require issue attached labels Dec 3, 2023
@nopcoder nopcoder disabled auto-merge December 4, 2023 19:08
@nopcoder
Copy link
Contributor

nopcoder commented Dec 6, 2023

force merge as external fork currently doesn't run all the required checks

@nopcoder nopcoder merged commit 3edf21b into treeverse:master Dec 6, 2023
35 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
include-changelog PR description should be included in next release changelog minor-change Used for PRs that don't require issue attached
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants