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

Refactor of config pkg #104

Closed
capri-xiyue opened this issue Aug 2, 2022 · 2 comments · Fixed by #125
Closed

Refactor of config pkg #104

capri-xiyue opened this issue Aug 2, 2022 · 2 comments · Fixed by #125
Labels
cleanup Refactoring, cleanup... good first issue Good for newcomers
Milestone

Comments

@capri-xiyue
Copy link
Contributor

capri-xiyue commented Aug 2, 2022

TL;DR

There are some duplicates when loading config from env, do validation and set defaults.
https://github.com/abcxyz/jvs/blob/main/pkg/config/public_key_config.go#L43 and

func loadCryptoConfigFromLookuper(ctx context.Context, b []byte, lookuper envconfig.Lookuper) (*CryptoConfig, error) {
.

We can do refactor to avoid duplicates.

Detailed design

Create a common function which handles loading config from env with any parameter. The caller of the common function need to pass the pointer of specific config like PublicKeyConfig or CryptoConfig as the value of any parameter.

Alternatives considered

No response

Additional information

No response

@capri-xiyue capri-xiyue added enhancement New feature or request good first issue Good for newcomers labels Aug 2, 2022
@yolocs yolocs added cleanup Refactoring, cleanup... and removed enhancement New feature or request labels Aug 31, 2022
@yolocs
Copy link
Contributor

yolocs commented Sep 15, 2022

Added a lib in pkg: abcxyz/pkg#33

@yolocs
Copy link
Contributor

yolocs commented Sep 19, 2022

Adopt pkg/cfgloader wherever applicable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Refactoring, cleanup... good first issue Good for newcomers
Development

Successfully merging a pull request may close this issue.

2 participants