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

Align codec interface to exclusively use packers #3359

Merged

Conversation

StephenButtolph
Copy link
Contributor

Why this should be merged

Because the interface doesn't include a Marshal function, it feels very weird to have an Unmarshal function.

How this works

Removes the Unmarshal function from the interface.

How this was tested

  • Existing CI

@StephenButtolph StephenButtolph added the cleanup Code quality improvement label Sep 4, 2024
@StephenButtolph StephenButtolph self-assigned this Sep 4, 2024
@StephenButtolph StephenButtolph added this to the v1.11.12 milestone Sep 4, 2024
Copy link
Contributor

@tsachiherman tsachiherman left a comment

Choose a reason for hiding this comment

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

it would impact non-genericCodec unmarshalers as well. I don't know if we have any.
generally speaking, this would prevent serializing a series of objects into the same byte array in a sequence. This might be ok if we don't have any such usage.

@StephenButtolph
Copy link
Contributor Author

it would impact non-genericCodec unmarshalers as well. I don't know if we have any. generally speaking, this would prevent serializing a series of objects into the same byte array in a sequence. This might be ok if we don't have any such usage.

Good point, we don't have any such usage. I think going forward this PR (and it's parent) standardize this a bit. After these PRs the manager enforces a codec version with no trailing data, the internal codecs don't know about the codec version and support reading from a sequence.

@StephenButtolph StephenButtolph merged commit 4b535d6 into codec-unmarshal-from Sep 4, 2024
19 checks passed
@StephenButtolph StephenButtolph deleted the codec-unmarshal-from-followup branch September 4, 2024 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code quality improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants