-
Notifications
You must be signed in to change notification settings - Fork 3
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
WebhookServiceの構造体定義+NewServerで読み込みを行うようにした #814
base: v2
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v2 #814 +/- ##
==========================================
- Coverage 25.26% 25.24% -0.02%
==========================================
Files 147 147
Lines 30583 30737 +154
==========================================
+ Hits 7726 7761 +35
- Misses 21980 22090 +110
- Partials 877 886 +9 ☔ View full report in Codecov by Sentry. |
router/router.go
Outdated
type WebHookService struct { | ||
webhookSecret string; | ||
webhookChannelId string; | ||
webhookId string; | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
構造体の概形はあってます 👍
ただ、フィールドが小文字始まりになってしまっているのでservice
パッケージ側からアクセスできません...
加えて、このPRではservice/webhook.go
にあるWebhookEventHandler
とRequestWebhook
を、ただの関数から作成した構造体のメソッドに置き換えることを目標としたいです。
// before
func WebhookEventHandler(...)
func RequestWebhook(...) error
// after
func (s WebHookService) WebhookEventHandler(...)
func (s WebHookService) RequestWebhook(...) error
ので、構造体の定義はrouter
側ではなくservice
側に置いて欲しいです。NewWebhookService
のような関数をservice/webhook.go
に生やして、router
側ではこの関数を使ってWebhookService
を作成すると丸そうだなと思ってます。 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(あと細かいですが、WebHook
ではなくWebhook
に揃えて欲しいです)
WebhookServiceの構造体定義+NewServerで読み込みを行うようにした