Skip to content
This repository was archived by the owner on Aug 24, 2022. It is now read-only.

Conversation

@pkadej
Copy link
Contributor

@pkadej pkadej commented Apr 13, 2022

PMM-4879

Build: SUBMODULES-2465

Build will fail because I cannot use forked version of api (pmm) as dependency in mod.go. To acomplish this, api needs be merged and version of pmm api should be bumped.

@it-percona-cla
Copy link

it-percona-cla commented Apr 13, 2022

CLA assistant check
All committers have signed the CLA.

}

// ParseDefaultsFile parses given defaultsFile. It returns the database specs.
func (d *DefaultsFile) ParseDefaultsFile(req *agentpb.ParseDefaultsFileRequest) *agentpb.ParseDefaultsFileResponse {
Copy link
Contributor

Choose a reason for hiding this comment

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

let's create one more struct like DefaultsFileParser with no fields and make this method part of it.
because TBH it looks weird that DefaultsFile has method which calls another function which returns DefaultsFile and fields of it used. So I think having separate struct will make code cleaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.
Just created "Parser" struct because package is "defaultsfile" and with DefaultsFileParser linter raises error about stuttering. type name will be used as defaultsfile.DefaultsFileParser by other packages, and that stutters

)

// DefaultsFile is a struct with database specs fetched from file.
type DefaultsFile struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

in case of new struct DefaultFileParser, DefaultsFile can be private.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great remark. Thank you, changed.


return &defaultsFile{
username: cfgSection.Key("user").String(),
password: cfgSection.Key("password").String(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Later password need to be encrypted, please create related ticket so that we don't forget about it

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants