-
Notifications
You must be signed in to change notification settings - Fork 69
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(notifier): add validation notifier config #1099
Conversation
/build |
helpers.go
Outdated
@@ -250,3 +252,9 @@ func MergeToSorted[T Comparable](arr1, arr2 []T) ([]T, error) { | |||
|
|||
return merged, nil | |||
} | |||
|
|||
// ValidateConfig is a generic function needed to validate the config structure using validator. | |||
func ValidateConfig(cfg any) error { |
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.
если это в херпере живет
это же не обязательно валидейт конфиг может быть?
ну это наброс только ради нейминга, что думаете?
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.
Куда лучше его тогда засунуть? У нас в хелперах живут функции, которые обобщенные и могут переиспользоваться в разных модулях
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.
мэйба в район cmd
или нэйминг поправить
я бы парней еще послушала
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.
а зачем в cmd? Он же вообще никак с cmd не связан и используется в сендерах. А касательно нейминга даже хз, как лучше, может ValidateConfigStruct
, тк мы именно структуру валидируем?
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.
@AleksandrMatsko @Tetrergeru давайте ваше мнение тоже узнаем
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.
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.
Мне кажется, что в хелперсах норм. Может, разве что, завести какой-то отдельный файлик поближе к сендерам, чтобы не всё в кучу
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.
то есть сделать helpers/util внутри с папками сендеров?
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.
Да, примерно такое имел в виду
Build and push Docker images with tag: 2024-10-07.8255890 |
1 similar comment
Build and push Docker images with tag: 2024-10-07.8255890 |
Add validation notifier config
Added validation of the notifier's senders config, required fields are now checked with validator