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

overall 3.0.3 review #28

Closed
wants to merge 1 commit into from
Closed

Conversation

hopeyen
Copy link

@hopeyen hopeyen commented Aug 12, 2024

This was mostly for my own educational purposes to become more familiar with the integration; I looked at all changes between v3.0.3 and eigenda-v3.0.3

  • added else if for methodName consistency ; in the case where both use4844 and useEigenDA, use4844 encoding will be used but the method name would currently be assigned to for EigenDA.
  • use switch for encodeAddBatch path (thinking it is more efficient but not a big deal in this scenario) , factored out encodeEigenDABatch to its own function
  • general q: could we move eigenda package to eigenda repo and give it some name like EigenDARollupUtil?

@hopeyen hopeyen added the good first issue Good for newcomers label Aug 12, 2024
@hopeyen hopeyen self-assigned this Aug 12, 2024
@bxue-l2 bxue-l2 requested review from epociask and bxue-l2 August 14, 2024 19:34
@hopeyen
Copy link
Author

hopeyen commented Aug 22, 2024

^the only critical thing here is already included within the PR#32, so closing as there's no significance to this PR

@hopeyen hopeyen closed this Aug 22, 2024
@hopeyen hopeyen deleted the hope/overall-3.0.3-review branch August 22, 2024 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant