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

refactor: modern project structure in launch_screen #76

Merged
merged 3 commits into from
Nov 6, 2024

Conversation

FantasyRL
Copy link
Member

refer to #71

Copy link
Member

@ozline ozline left a comment

Choose a reason for hiding this comment

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

把全部修改文件的 import 检查一遍,遵循下面规范

  1. golang 内置包
  2. 外部包
  3. 隶属于本 org(west2-online)的包

除此之外,注意一下 cache、db 的一些通用函数,前面一些函数写重复了

@@ -23,21 +23,28 @@ import (
"github.com/cloudwego/netpoll"
etcd "github.com/kitex-contrib/registry-etcd"

"github.com/west2-online/fzuhelper-server/pkg/base"
Copy link
Member

Choose a reason for hiding this comment

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

import 遵循规范

Copy link
Member

Choose a reason for hiding this comment

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

svr.Run()前注册退出函数,e.g.

server.RegisterShutdownHook(clientSet.Close)

@@ -19,18 +19,22 @@ package service
import (
"context"

"github.com/west2-online/fzuhelper-server/pkg/utils"

Copy link
Member

Choose a reason for hiding this comment

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

遵循 import 规范

"github.com/west2-online/fzuhelper-server/kitex_gen/model"
model2 "github.com/west2-online/fzuhelper-server/pkg/db/model"
Copy link
Member

Choose a reason for hiding this comment

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

import 规范修正

@@ -20,25 +20,26 @@ import (
"fmt"
"time"

"github.com/west2-online/fzuhelper-server/pkg/db/model"

Copy link
Member

Choose a reason for hiding this comment

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

import 规范修正

"github.com/west2-online/fzuhelper-server/pkg/cache"
"github.com/west2-online/fzuhelper-server/pkg/db"
"github.com/west2-online/fzuhelper-server/pkg/db/model"

Copy link
Member

Choose a reason for hiding this comment

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

import 规范修正

"github.com/west2-online/fzuhelper-server/pkg/db"
"github.com/west2-online/fzuhelper-server/pkg/db/model"
"github.com/west2-online/fzuhelper-server/pkg/utils"

Copy link
Member

Choose a reason for hiding this comment

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

import 规范修正

todo:use cache.go
*/

func (c *CacheLaunchScreen) IsLaunchScreenCacheExist(ctx context.Context, key string) bool {
Copy link
Member

Choose a reason for hiding this comment

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

删掉这个玩意,已经有通用函数了

}

func IsLastLaunchScreenIdCacheExist(ctx context.Context) bool {
return RedisClient.Exists(ctx, constants.LastLaunchScreenIdKey).Val() == 1
func (c *CacheLaunchScreen) IsLastLaunchScreenIdCacheExist(ctx context.Context) bool {
Copy link
Member

Choose a reason for hiding this comment

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

这个删掉,用通用函数

@ozline
Copy link
Member

ozline commented Nov 3, 2024

@jiuxia211 @SchwarzSail 快来review然后合了

jiuxia211
jiuxia211 previously approved these changes Nov 3, 2024
@@ -52,7 +60,7 @@ func (s *LaunchScreenServiceImpl) CreateImage(stream launch_screen.LaunchScreenS
req.Image = bytes.Join([][]byte{req.Image, fileReq.Image}, []byte(""))
}

pic, err := service.NewLaunchScreenService(stream.Context()).CreateImage(req)
pic, err := service.NewLaunchScreenService(stream.Context(), s.ClientSet).CreateImage(req)
Copy link
Member

Choose a reason for hiding this comment

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

这边在注入依赖的时候是把 ClientSet 全部注入进去吗,还是注入 service 所需要的 client?

Copy link
Member

Choose a reason for hiding this comment

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

我偏向于后者,需要你在 service 定义出你所需要的 client 吧(

@ozline ozline changed the title refactor:modern project structure in launch_screen refactor: modern project structure in launch_screen Nov 4, 2024
@ozline
Copy link
Member

ozline commented Nov 4, 2024

@FantasyRL 修一下 ci,帮你解决冲突了,后面应该也由你自己来解决冲突,基本功了属于是

@FantasyRL
Copy link
Member Author

@jiuxia211 cc

Copy link
Member

@ozline ozline left a comment

Choose a reason for hiding this comment

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

LGTM

@ozline ozline merged commit 054ec7c into west2-online:main Nov 6, 2024
6 checks passed
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.

4 participants