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.

Sign up for GitHub

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

Jump to bottom

feat!: add ignorePadding option to ParseShares #1662

Merged
merged 1 commit into from
Apr 28, 2023
Merged

feat!: add ignorePadding option to ParseShares #1662

merged 1 commit into from
Apr 28, 2023

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Apr 26, 2023

Closes #1649

@rootulp rootulp added this to the Mainnet milestone Apr 26, 2023
@rootulp rootulp self-assigned this Apr 26, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #1662 (f18d9f0) into main (f31ee58) will increase coverage by 0.10%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##             main    #1662      +/-   ##
==========================================
+ Coverage   51.56%   51.66%   +0.10%     
==========================================
  Files          95       95              
  Lines        5954     5979      +25     
==========================================
+ Hits         3070     3089      +19     
- Misses       2570     2574       +4     
- Partials      314      316       +2
Impacted Files Coverage Δ
pkg/shares/share_sequence.go 75.00% <62.50%> (-2.78%) ⬇️
pkg/shares/parse.go 76.19% <72.72%> (+3.96%) ⬆️

... and 1 file with indirect coverage changes

@rootulp rootulp requested a review from vgonkivs April 26, 2023 19:13
@rootulp rootulp marked this pull request as ready for review April 26, 2023 19:14
@rootulp rootulp requested a review from cmwaters April 28, 2023 13:08
@rootulp rootulp merged commit 318d785 into celestiaorg:main Apr 28, 2023
25 checks passed
@rootulp rootulp deleted the rp/parse-shares-ignore-padding branch April 28, 2023 13:29
@rootulp rootulp mentioned this pull request May 24, 2023
Merged
rootulp added a commit to rootulp/celestia-app that referenced this pull request May 24, 2023
@rootulp
refactor: use the exact same code that is on main
ac65d16 
See celestiaorg#1662
@rootulp rootulp mentioned this pull request May 24, 2023
Merged
rootulp added a commit that referenced this pull request May 26, 2023
@rootulp
test: add tests for validSequenceLen (#1817)
7e93692 
Part of investigating
#1816

Note: an implementation change isn't necessary b/c the bug was already
fixed by #1662
evan-forbes pushed a commit that referenced this pull request May 26, 2023
@rootulp @evan-forbes
test: add tests for validSequenceLen (#1817)
bc30bbd 
Part of investigating
#1816

Note: an implementation change isn't necessary b/c the bug was already
fixed by #1662
evan-forbes pushed a commit that referenced this pull request May 26, 2023
@rootulp @evan-forbes
test: add tests for validSequenceLen (#1817)
0494828 
Part of investigating
#1816

Note: an implementation change isn't necessary b/c the bug was already
fixed by #1662
cmwaters pushed a commit to celestiaorg/go-square that referenced this pull request Dec 14, 2023
@rootulp
test: add tests for validSequenceLen (#1817)
e217530 
Part of investigating
celestiaorg/celestia-app#1816

Note: an implementation change isn't necessary b/c the bug was already
fixed by celestiaorg/celestia-app#1662
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmwaters cmwaters cmwaters approved these changes

@evan-forbes evan-forbes evan-forbes approved these changes

@vgonkivs vgonkivs Awaiting requested review from vgonkivs

Assignees

@rootulp rootulp

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make ParseShares skip padding shares
4 participants
@rootulp @codecov-commenter @cmwaters @evan-forbes