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

Create payload->blob type system #1120

Merged
merged 43 commits into from
Feb 19, 2025
Merged

Create payload->blob type system #1120

merged 43 commits into from
Feb 19, 2025

Conversation

litt3
Copy link
Contributor

@litt3 litt3 commented Jan 16, 2025 •
edited
Loading

Why are these changes needed?

Checks

  • I've made sure the tests are passing. Note that there might be a few flaky tests, in that case, please comment that they are not relevant.
  • I've checked the new test coverage and the coverage percentage didn't drop.
  • Testing Strategy
    • Unit tests
    • Integration tests
    • This PR is not tested :(

@litt3 litt3 requested review from samlaf and bxue-l2 January 16, 2025 20:44
@litt3 litt3 self-assigned this Jan 16, 2025
litt3 added 15 commits January 17, 2025 09:11
@litt3
Revert "Change name to ProtoBlob"
856f5fc 
This reverts commit 422c2a8.

Signed-off-by: litt3 <[email protected]>
@litt3
Rename PayloadHeader to EncodedPayloadHeader
1eeec85 
Signed-off-by: litt3 <[email protected]>
@litt3
Merge branch 'master' into payload-blob-type-system
a30a4ad
@litt3
Rename to PayloadEncodingVersion
5eab8dd 
Signed-off-by: litt3 <[email protected]>
@litt3
Do a bunch of cleanup
132e8de 
Signed-off-by: litt3 <[email protected]>
@litt3
Fix PayloadVersion naming
da63536 
Signed-off-by: litt3 <[email protected]>
@litt3
Merge branch 'master' into payload-blob-type-system
3016cac
@litt3
Use new conversion method
9f98462 
Signed-off-by: litt3 <[email protected]>
@litt3
Add better descriptions to codec utils
58b76c1 
Signed-off-by: litt3 <[email protected]>
@litt3
Do test work
916573f 
Signed-off-by: litt3 <[email protected]>
@litt3
Add length checks
7eed618 
Signed-off-by: litt3 <[email protected]>
@litt3
Merge branch 'master' into payload-blob-type-system
c3a6291 
Signed-off-by: litt3 <[email protected]>
@litt3
Write test for power of 2 util
b604c82 
Signed-off-by: litt3 <[email protected]>
@litt3
Write more tests
2ef8e99 
Signed-off-by: litt3 <[email protected]>
@litt3
Finish utils tests
13d16c7 
Signed-off-by: litt3 <[email protected]>
@litt3 litt3 requested a review from anupsv February 12, 2025 16:46
@litt3 litt3 requested a review from cody-littley February 12, 2025 18:35
samlaf
samlaf reviewed Feb 12, 2025
View reviewed changes
Copy link
Contributor

@samlaf samlaf left a comment •
edited
Loading

Choose a reason for hiding this comment

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

First pass. Still have more to review but tired.

cody-littley
cody-littley reviewed Feb 13, 2025
View reviewed changes
litt3 added 10 commits February 13, 2025 13:10
@litt3 litt3 requested a review from samlaf February 18, 2025 17:58
@litt3 litt3 requested a review from cody-littley February 18, 2025 18:37
This was referenced Feb 19, 2025
Merged
Closed
@litt3 litt3 requested a review from cody-littley February 19, 2025 19:29
samlaf
samlaf approved these changes Feb 19, 2025
View reviewed changes
Copy link
Contributor

@samlaf samlaf left a comment

Choose a reason for hiding this comment

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

LGTM except for the codecs/v2 package name. If swapping those around makes sense, please do that before merging. Otherwise rest looks awesome, really happy with this new code!

@litt3 litt3 merged commit 412342c into master Feb 19, 2025
12 checks passed
@litt3 litt3 deleted the payload-blob-type-system branch February 19, 2025 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samlaf samlaf samlaf approved these changes

@bxue-l2 bxue-l2 Awaiting requested review from bxue-l2

@anupsv anupsv Awaiting requested review from anupsv

@cody-littley cody-littley Awaiting requested review from cody-littley

Assignees

@litt3 litt3

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@litt3 @samlaf @cody-littley