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

[Feature] Add nested struct/*struct and embedded struct #8

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

jakefolio
Copy link

This PR adds support for nested structs, struct pointers, and embedded struct. Since this PR: #2 looks to have gone silent, I went ahead and made the requested changes (documentation, tests, etc.)

@ianlopshire
Copy link
Owner

ianlopshire commented Dec 16, 2020

@jakefolio Thanks for taking the time to submit this! I'll try to carve out some time this week to give this a proper review.

At first glance I have some questions:

How are paths handled for nested structs? To make nested structs really useful I think they must inherit the path from the parent struct.

type DBConfig struct {
    User     string `ssm:"user"` 
    Password string `ssm:"password"` 
}

type Config struct {
    PrimaryDB   DBConfig `ssm:"db/primary"` 
    SecondaryDB DBConfig `ssm:"db/secondary"` 
}

Does this implementation support nesting structs arbitrarily deep?

@jakefolio
Copy link
Author

Please correct me if I'm wrong, but I believe you're asking about SSM path inheritance? If so, I believe your example above has the expectation of DBConfig.Password having the SSM path of db/primary/password and db/secondary/password, correct?

This PR does not include that type of nesting, but that is an interesting feature that I believe is beyond the scope of the PR.

This PR allows for the parsing of nested struct and struct pointers like the example in the test:
https://github.com/ianlopshire/go-ssm-config/pull/8/files#diff-5004671fb5eff27da0c3f2ec0bd1cb5a250f7287004872e8f5f125073e86606dR34

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.

2 participants