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

[FR] Get rid of conf as shared global state #47

Closed
mtlynch opened this issue Jan 3, 2025 · 1 comment · Fixed by #48
Closed

[FR] Get rid of conf as shared global state #47

mtlynch opened this issue Jan 3, 2025 · 1 comment · Fixed by #48

Comments

@mtlynch
Copy link
Contributor

mtlynch commented Jan 3, 2025

What

I'd like to continue #46 by removing the accessors from conf and read the configuration from a single place in the main package, like this:

config, err := conf.Load()
if err != nil {
  log.Fatalf("invalid config: %v", err)
}
repo.Init(config.DB)

go pull.NewPuller(repo.NewFeed(repo.DB), repo.NewItem(repo.DB)).Run()

api.Run(api.apiParams{
  Password: config.Password,
  Host: config.Host,
  Port: config.Port,
  TLSCert: config.TLSCert,
  TLSKey: config.TLSKey,
})

I'm happy to implement this if it sounds good to you.

Why

Shared global state makes it hard to understand how different components fit together. Eliminating the global Conf struct makes it easier to unit test code because tests can pass values directly to the functions they exercise rather than setting the Conf global in advance to influence behavior.

(Optional) How

See code example above.

@0x2E
Copy link
Owner

0x2E commented Jan 4, 2025

Make sense. Would love to merge if you implement it :D

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 a pull request may close this issue.

2 participants