Skip to content

Conversation

@raphala
Copy link
Contributor

@raphala raphala commented Nov 12, 2025

No description provided.

@raphala raphala marked this pull request as ready for review November 14, 2025 10:59
Copy link
Member

@jkbe jkbe left a comment

Choose a reason for hiding this comment

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

Some naming issues are similar to #385, e.g. there are a couple of results in files that match *consumerproducer* for /application.?id/ (should likely be group ID) and also streams (should often be consumer producer)

*/
@RequiredArgsConstructor
@With
public class SerializerDeserializerConfig implements SerializationConfig {
Copy link
Member

Choose a reason for hiding this comment

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

We noticed during implementation of our first ConsumerProducers that perhaps switching the name parts around to DeserializerSerializerConfig makes sense since in the "data flow", input records are deserialized before output records are serialized. WDYT? In case you agree, we should also switch around the args in the constructor below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Valid point on the data flow. I don't have a strong opinion, but my slight preference is still SerializerDeserializerConfig just to match the industry-standard SerDe, but I am open to switching it if you feel strongly about switching the naming.

README.md Outdated
Comment on lines 317 to 318
// your logic
consumedValue -> consumedValue
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps also produce here in the example?

@raphala raphala requested a review from jkbe November 28, 2025 13:45
@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants