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

feature(encoder): Implement Iterator over DaBlob for EncodedData #952

Merged
merged 3 commits into from
Dec 19, 2024

Conversation

AlejandroCabeza
Copy link
Contributor

@AlejandroCabeza AlejandroCabeza commented Dec 18, 2024

1. What does this PR implement?

Implement Iterator over DaBlob for EncodedData
Closes: #943

2. Does the code have enough context to be clearly understood?

Yes

3. Who are the specification authors and who is accountable for this PR?

@AlejandroCabeza

4. Is the specification accurate and complete?

N/A

5. Does the implementation introduce changes in the specification?

N/A

Checklist

Warning

Do not merge the PR if any of the following is missing:

  • 1. Description added.
  • 2. Context and links to Specification document(s) added.
  • 3. Main contact(s) (developers and specification authors) added
  • 4. Implementation and Specification are 100% in sync including changes. This is critical.
  • 5. Link PR to a specific milestone.

Copy link
Collaborator

@danielSanchezQ danielSanchezQ left a comment

Choose a reason for hiding this comment

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

Implementation looks ok, but having an iteration state inside that structure is wrong. What happens if I need 5 iterators over it? You will need to add 5 states then...

@danielSanchezQ danielSanchezQ self-requested a review December 19, 2024 12:13
Copy link
Collaborator

@danielSanchezQ danielSanchezQ left a comment

Choose a reason for hiding this comment

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

Looking good, thanks!
We can use this in the executor now. Probably something to do when merging this. Please add an issue and attach to the ongoing iteration.

@AlejandroCabeza AlejandroCabeza merged commit 739df17 into master Dec 19, 2024
5 checks passed
@AlejandroCabeza AlejandroCabeza deleted the feature/encoded-data-iterator branch December 19, 2024 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement iter (da_blob) over EncodedData
2 participants