Skip to content

Conversation

@deryrahman
Copy link
Member

@deryrahman deryrahman commented Feb 24, 2022

  • remove config/interface.go
  • remove config.Optimus getters

@coveralls
Copy link

coveralls commented Feb 24, 2022

Pull Request Test Coverage Report for Build 1897418940

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 72.399%

Totals Coverage Status
Change from base Build 1862408092: 0.0%
Covered Lines: 5115
Relevant Lines: 7065

💛 - Coveralls

@kushsharma
Copy link
Member

Hi @deryrahman, although this looks good, can you please help me understand the reasoning behind why we are doing this?

@deryrahman
Copy link
Member Author

deryrahman commented Feb 25, 2022

Hi @kushsharma, As of now, since we only have 1 config implementation (which is config.Optimus) we don't need the interface. We also saw that the config is only data structure and have no behavior. And all the fields of conf.Optimus struct is exported (public) so we don't see the urgency of having getter. But if there's a strong reason to use getter, then we can discuss. cmiiw @sbchaos @sravankorumilli

@kushsharma
Copy link
Member

Hi @kushsharma, As of now, since we only have 1 config implementation (which is config.Optimus) we don't need the interface. We also saw that the config is only data structure and have no behavior. And all the fields of conf.Optimus struct is exported (public) so we don't see the urgency of having getter. But if there's a strong reason to use getter, then we can discuss. cmiiw @sbchaos @sravankorumilli

Make sense, unless we have more than one implementation it is not needed as of now. The idea of having the interface was if we needed to inject a provider like etcd store or gcloud runtimevars for runtime changes but I agree that is unlikely in near future.

dery.ahaddienata added 3 commits February 25, 2022 14:07
refactor: inject host config on listSecret function

refactor: inject host config on updateSecret function

refactor: inject host config on registerSecret function

refactor: inject host config on runReplayRequest function

refactor: inject host config on printReplayExecutionTree function

refactor: inject host config on runBackupRequest function

refactor: inject host config on runBackupDryRunRequest function
@deryrahman deryrahman force-pushed the remove-config-interface branch from 2321763 to 5dc527a Compare February 25, 2022 07:33
@ravisuhag ravisuhag merged commit 1ec74c3 into raystack:main Feb 25, 2022
@ravisuhag ravisuhag linked an issue Feb 25, 2022 that may be closed by this pull request
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.

Remove interface for Config provider and use the Config struct

5 participants