Skip to content

Conversation

@kelevis
Copy link

@kelevis kelevis commented Feb 4, 2024

No description provided.

@kelevis
Copy link
Author

kelevis commented Feb 4, 2024

config tips and config distinctions.
help reminder.

Copy link
Owner

@qingwave qingwave left a comment

Choose a reason for hiding this comment

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

Thanks for your PR, seems this PR want to support env mode(test, dev, prod), there are some questions here:

  1. app.yaml is the default config for the project, if users need custom config, just copy the app.yaml and modify it.
  2. if we need different configs for different env, but dev_config.yaml and prod_config.yaml are same at this time point.

I suggest we donot need to change the app.yaml, just add some details(how to config in different configs) in the ReadMe.

If you have some better ideas, feel free to comments.

thanks you again.

logger.Fatalf("Failed to parse config: %v", err)
}

if *appConfig != defaultConfigPath {
Copy link
Owner

Choose a reason for hiding this comment

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

It's better print config path rather than a info

if appConfig != nil {
  logger.Info(App config from "%s", *appConfig)
}

return version
}

func ConfigPathPrint() {
Copy link
Owner

Choose a reason for hiding this comment

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

We donot need to change this file, just print info in main.go

@@ -0,0 +1,45 @@
server:
env: "debug"
address: "127.0.0.1"
Copy link
Owner

Choose a reason for hiding this comment

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

If this a production config, we need modify some params.
env: release
address: 0.0.0.0

var (
printVersion = flag.Bool("v", false, "print version")
appConfig = flag.String("config", "config/app.yaml", "application config path")
defaultConfigPath = "config/dev_config.yaml"
Copy link
Owner

Choose a reason for hiding this comment

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

defaultConfigPath -> defaultConfig

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