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

update: sync done by the config itself #3

Merged
merged 2 commits into from
Jan 26, 2023

Conversation

cappellinsamuele
Copy link
Contributor

Refactor of the main loop to improve tests.

Signed-off-by: cappellinsamuele <cappellinsamuele@gmail.com>
os.Exit(1)
}

func (g *GithubConfig) Loop(client Client, provider SecretsProvider) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
func (g *GithubConfig) Loop(client Client, provider SecretsProvider) error {
func (g *GithubConfig) Loop(client *Client, provider SecretsProvider) error {

I know it can be confusing, but Client would need to be a pointer here, whereas SecretsProvider does not. The reason is that SecretsProvider is an interface type, which means that under the hood it is already "pointing to something". Since Config is a struct instead, passing it not as a pointer would cause the whole data struct to be copied. Not a big deal but also not ideal.

Copy link
Collaborator

@FedeDP FedeDP Jan 25, 2023

Choose a reason for hiding this comment

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

I think Loop should take ActionsVarsService and ActionsSecretsService as input in place of Client, so that we can use it in tests mocking both interfaces.

Copy link
Collaborator

@FedeDP FedeDP left a comment

Choose a reason for hiding this comment

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

I'd also refactor out the

conf := poiana.GithubConfig{}
	err := conf.Decode(strings.NewReader(sampleYAML))
	if err != nil {
		fail(err.Error())
	}
	provider, _ := poiana.NewMockSecretsProvider(map[string]string{
		"TEST_SECRET_KEY2": "ciaociaociao2",
		"TEST_SECRET_KEY":  "ciaociaociao",
})

To a couple of ctors like

config.FromFile(yamlFile)
config.FromData(yamlData)

WDYT?
We can do it in a later step though ;)

Copy link
Collaborator

@FedeDP FedeDP left a comment

Choose a reason for hiding this comment

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

/approve

Signed-off-by: cappellinsamuele <cappellinsamuele@gmail.com>
@FedeDP FedeDP merged commit 9c4d1e6 into main Jan 26, 2023
@FedeDP FedeDP deleted the update/conf_sync_encapsultation branch January 26, 2023 13:38
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