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

config structs require Serialize #1336

Merged
merged 2 commits into from
Sep 18, 2023

Conversation

rukai
Copy link
Member

@rukai rukai commented Sep 13, 2023

This PR exposes all our config structs in the public interface and requires that they implement Serialize.
Additionally a serialize method is added to Topology which serializes the config into yaml.
The goal is to allow the programmatic generation of topology.yaml files.

Users could always just use some kind of yaml templating but if the code doing the generation is rust then this change will allow for a stronger typed solution.

This change comes at the cost of:

  • a small increase to compile times, although I couldnt measure any change on my machine.
  • larger public API surface to maintain
  • custom transform config implementations now need to implement serialize as well deserialize.

We should consider this PR from a users perspective but it will also enable #1332

@rukai rukai force-pushed the transform-configs-require-serialize branch 2 times, most recently from 7b989de to 3361d97 Compare September 13, 2023 23:59
@rukai rukai requested a review from conorbros September 14, 2023 00:55
@rukai rukai force-pushed the transform-configs-require-serialize branch from 3361d97 to 261793b Compare September 14, 2023 21:48
@rukai rukai enabled auto-merge (squash) September 18, 2023 11:11
@rukai rukai merged commit 5b18599 into shotover:main Sep 18, 2023
37 checks passed
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