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

feat: init server project #5

Merged
merged 1 commit into from
Jun 20, 2023
Merged

feat: init server project #5

merged 1 commit into from
Jun 20, 2023

Conversation

IRONICBo
Copy link
Collaborator

🔍 What type of PR is this?

/kind feature

👀 What this PR does / why we need it:

init server project

🅰 Which issue(s) this PR fixes:

issue: #3

Fixes # Add server base project.

📝 Special notes for your reviewer:

🎯 Describe how to verify it

📑 Additional documentation e.g., RFC, notion, Google docs, usage docs, etc.:

@pull-request-size pull-request-size bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 20, 2023
@pull-request-size
Copy link

Whoa! Easy there, Partner!

This PR is too big. Please break it up into smaller PRs.

@cubxxw cubxxw merged commit 097f542 into openimsdk:main Jun 20, 2023
Secure: false,
},
)
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if err != nil {
if err != nil {
log.Panicf("Minio", err.Error(), " Open Bucket failed ", endpoint)
}

If an error occurs and the code simply logs the error message using the log.Panicf() function and exits, it is not friendly enough to consider returning the error message to the caller and letting the caller decide how to handle the error.

},
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

whether or not

type Config struct {
    App    AppConfig
    JWT    JWTConfig
    Server ServerConfig
    Mysql  MysqlConfig
    Redis  RedisConfig
    Minio  MinioConfig
}

type AppConfig struct {
    Version string `mapstructure:"version"`
    Debug   bool   `mapstructure:"debug"`
    LogFile string `mapstructure:"log_file"`
}

viper library to read configuration files

viper.SetConfigFile(configPath)

if err := viper.ReadInConfig(); err != nil {
panic(fmt.Errorf("fatal error config file: %s", err))
Copy link
Contributor

Choose a reason for hiding this comment

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

log record?

)

func init() {
configPath := "./config.yaml"
Copy link
Contributor

Choose a reason for hiding this comment

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

hard coded ~

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I need to specify a path to read the configuration file, do I need to consider reading the configuration file from the command line?

config.ConfigInit()

// logo
_logo := `
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you can, go to https://github.com/avelino/awesome-go to find more pattern, command line beautification output what ha ha ha ha ha

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good, I will try to choose one.

// Licensed under the MIT License (the "License");
// you may not use this file except in compliance with the License.

package log
Copy link
Contributor

Choose a reason for hiding this comment

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

logrus vs zap

Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest implementing an authentication mechanism in Redis to enhance the security of its connections.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I will update it.

"context"
"fmt"
"io"

Copy link
Contributor

Choose a reason for hiding this comment

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

there is one more spacce here

Copy link
Contributor

@PolarishT PolarishT left a comment

Choose a reason for hiding this comment

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

agree to merge

@IRONICBo IRONICBo mentioned this pull request Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants