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.

Sign up for GitHub

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

Jump to bottom

🔥 service層を消す #607

Merged
merged 12 commits into from
Apr 28, 2024
Merged

🔥 service層を消す #607

merged 12 commits into from
Apr 28, 2024

Conversation

H1rono
Copy link
Member

@H1rono H1rono commented Dec 3, 2023 •
edited
Loading

close #431

  • usecases/repositoryにinterfaceをどうにかする(移す or 消す or ...)
  • architecture.md修正(このPRじゃないかも)
  • test修正

@H1rono H1rono requested a review from ras0q December 3, 2023 10:09
@H1rono
Copy link
Member Author

H1rono commented Dec 3, 2023

まだUserHandlerだけ、WIPとして

@ras0q
Copy link
Member

ras0q commented Dec 3, 2023

ユニットテストが落ちてるのはserviceのモックを使ってるところをrepositoryのモックに変更する必要があるからですね

@ras0q
Copy link
Member

ras0q commented Dec 3, 2023

  1. rm services/user.go service/mock_service/mock_user.go
  2. go generate ./... (user service mockが生成されないことを確認)
  3. user service mockを消してエラーが出たところを修正

でいけるかな

@H1rono
Copy link
Member Author

H1rono commented Dec 4, 2023

@ras0q この方針で良さそうだったら他のserviceも消していきます

@ras0q
Copy link
Member

ras0q commented Dec 4, 2023

よさそう、お願いします！

@codecov Codecov
Copy link

codecov bot commented Dec 24, 2023 •
edited
Loading

Codecov Report

Attention: Patch coverage is 62.85714% with 39 lines in your changes are missing coverage. Please review.

Project coverage is 61.06%. Comparing base (031ecf2) to head (b87fc29).

Files Patch % Lines
interfaces/handler/project.go 20.68% 22 Missing and 1 partial ⚠️
interfaces/handler/contest.go 73.91% 3 Missing and 3 partials ⚠️
infrastructure/injector.go 0.00% 5 Missing ⚠️
interfaces/handler/group.go 83.33% 2 Missing and 1 partial ⚠️
interfaces/handler/event.go 85.71% 1 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #607      +/-   ##
==========================================
- Coverage   66.67%   61.06%   -5.61%     
==========================================
  Files          64       54      -10     
  Lines        4780     4269     -511     
==========================================
- Hits         3187     2607     -580     
- Misses       1545     1611      +66     
- Partials       48       51       +3
Flag Coverage Δ
integration 61.06% <62.85%> (-5.61%) ⬇️
unit 61.06% <62.85%> (-5.61%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@H1rono H1rono force-pushed the remove-service branch 2 times, most recently from 39d0fa7 to b1bed40 Compare February 8, 2024 12:26
ras0q
ras0q reviewed Apr 18, 2024
View reviewed changes
Copy link
Member

@ras0q ras0q left a comment

Choose a reason for hiding this comment

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

いくつか

interfaces/handler/contest.go Show resolved Hide resolved
interfaces/handler/testutil_test.go Outdated Show resolved Hide resolved
interfaces/handler/user.go
"github.com/gofrs/uuid"
"github.com/labstack/echo/v4"
"github.com/traPtitech/traPortfolio/usecases/repository"
)

type UserHandler struct {
s service.UserService
user repository.UserRepository
Copy link
Member

Choose a reason for hiding this comment

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

移行した時のやつだと思うけどrepositoryを単数使うところと複数使うところで命名が違うのをどうにかしたい
冗長にuserRepoとかでもいい気もする

Copy link
Member Author

Choose a reason for hiding this comment

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

traPortfolio/interfaces/handler/contest.go

Lines 17 to 19 in 6442b2c

type ContestHandler struct {
r repository.ContestRepository
}

traPortfolio/interfaces/handler/user.go

Lines 15 to 18 in 4e547ae

type UserHandler struct {
user repository.UserRepository
event repository.EventRepository
}

ここ？1文字の方をcontestとかにrenameしたら良さそうに見えるけど

Copy link
Member

Choose a reason for hiding this comment

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

まさにそこです
まあcontestRepo(sitory)が冗長そうならcontestでいい気もする
関連?: #573

Copy link
Member

Choose a reason for hiding this comment

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

それかMockRepositoryで実装してくれてるみたいにh.r.contestみたいに呼び出せるRepository構造体を生やすとか

Copy link
Member Author

Choose a reason for hiding this comment

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

Repositoryを生やすのはちょっと考えた方が良さそう、少なくともここではやらない
1文字のほうrenameします

@ras0q
Copy link
Member

ras0q commented Apr 27, 2024

@H1rono

@H1rono
Copy link
Member Author

H1rono commented Apr 27, 2024

う

@H1rono H1rono requested a review from ras0q April 28, 2024 01:14
ras0q
ras0q reviewed Apr 28, 2024
View reviewed changes
interfaces/handler/contest_test.go Outdated
Comment on lines 30 to 32
r := mock_repository.NewMockContestRepository(ctrl)
mr := MockRepository{nil, nil, r, nil, nil}
api := NewAPI(nil, nil, nil, nil, NewContestHandler(r), nil)
Copy link
Member

Choose a reason for hiding this comment

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

MockRepository{Contest: r}でよさそう
(下のNewAPIがこの書き方になっちゃってるのも見にくいけど)

interfaces/handler/user.go
"github.com/gofrs/uuid"
"github.com/labstack/echo/v4"
"github.com/traPtitech/traPortfolio/usecases/repository"
)

type UserHandler struct {
s service.UserService
user repository.UserRepository
Copy link
Member

Choose a reason for hiding this comment

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

それかMockRepositoryで実装してくれてるみたいにh.r.contestみたいに呼び出せるRepository構造体を生やすとか

@H1rono H1rono requested a review from ras0q April 28, 2024 02:23
ras0q
ras0q approved these changes Apr 28, 2024
View reviewed changes
Copy link
Member

@ras0q ras0q left a comment

Choose a reason for hiding this comment

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

👍🏻

@H1rono H1rono merged commit e79e290 into main Apr 28, 2024
7 of 9 checks passed
@H1rono H1rono deleted the remove-service branch April 28, 2024 03:46
@ras0q ras0q mentioned this pull request Apr 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ras0q ras0q ras0q approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

service層を消す
2 participants
@H1rono @ras0q