refactor(link): expose config package - #1378
Conversation
Signed-off-by: Gjermund Garaba <gjermund@garaba.net>
Signed-off-by: Gjermund Garaba <gjermund@garaba.net>
ad95da4 to
2f2fd03
Compare
Greptile SummaryThe PR exposes Link configuration as a public package while preserving existing loading, validation, defaults, and YAML behavior.
Confidence Score: 5/5The PR appears safe to merge; no concrete changed-code failure remains. The refactor preserves configuration defaults, validation, loading flags, path handling, and runtime consumption, while the E2E migration uses the same public model and canonical codec with updated serialization coverage. Important Files Changed
|
There was a problem hiding this comment.
Is this causing alot of friction for you in E2E development? I'm not super opposed to making this public - my main concern is that breaking changes seem likely as we continue development due to multiple reasons
AutoRelayConfigis already changing as the design is being fleshed out.- We want to simplify the relayer architecture and some configs currently reflect the way the relayer is designed. I could see
DispatchPollIntervalchanging. We also want to expose parts of the relayer and attestors as libraries and make it possible to define custom implementations of some of the interfaces likeProofGeneratorfor example - I think the configs will need to change to support that as well. - We've made a best guess at choosing configs that can be extended without breaking changes to support multiple light client types and multiple chain types. As we actually extend beyond just attestations and just EVM support we may find that we require breaking changes to support additional light client and chain types or there is a more ideal config structure.
Unless it's a huge inconvenience right now, I would consider waiting till we add QBFT LC support, auto-relay and finish defining the public interface for the relayer + attestor. Lmk what you think!
| // CollectComments builds TODO comments for every field in cfg that's left | ||
| // for the operator to fill in by hand, keyed by YAML path for | ||
| // MarshalYAMLWithComments. | ||
| func CollectComments(cfg Config) map[string]string { |
There was a problem hiding this comment.
I wouldn't want to make this public yet, not sure it's something we want to keep
Signed-off-by: Gjermund Garaba <gjermund@garaba.net>
fc7e7e9 to
b6f7f20
Compare
|
i understand, but technically we dont actually commit to it until we cut a release, and the config is mostly already a public api in the sense that it is what the user interacts with directly anyway. this is not critical though, just would make things slightly simpler to work with. That said, if you prefer to just keep this as it is until more has landed, I'm not very opposed to it. Doesn't create a lot of friction (yet?), just having to make sure that things are kept in sync. |
Summary
link/configlink/configTesting
make build-linkmake lintgo test ./...inlink/make test-harnessmake test-e2egit diff --check