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

amt: support start_at that is out of range in the first place #1901

Merged

Conversation

zhiqiangxu
Copy link
Contributor

@zhiqiangxu zhiqiangxu commented Sep 28, 2023

This issue is found here, instead of panic, we can just return.

This is useful when start_at is estimated which may be out of range.

@zhiqiangxu zhiqiangxu force-pushed the opt_for_each_while_ranged branch from da54ece to e7d1f27 Compare September 28, 2023 03:46
@codecov-commenter
Copy link

codecov-commenter commented Sep 28, 2023

Codecov Report

Merging #1901 (e7d1f27) into master (0ed0188) will decrease coverage by 0.01%.
The diff coverage is 66.66%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1901      +/-   ##
==========================================
- Coverage   75.73%   75.73%   -0.01%     
==========================================
  Files         152      152              
  Lines       15078    15084       +6     
==========================================
+ Hits        11420    11424       +4     
- Misses       3658     3660       +2     
Files Coverage Δ
ipld/amt/src/node.rs 88.86% <66.66%> (-0.08%) ⬇️

... and 1 file with indirect coverage changes

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

Nice catch! Could you add a quick test case to exercise this?

@Stebalien
Copy link
Member

(I'll merge this now so I can cut a bug-fix release for you)

@Stebalien Stebalien merged commit 6b58221 into filecoin-project:master Sep 28, 2023
@Stebalien
Copy link
Member

Nvm. I added a test in #1902.

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