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

Improve advanced mode #470

Merged
merged 3 commits into from
May 28, 2020
Merged

Improve advanced mode #470

merged 3 commits into from
May 28, 2020

Conversation

litleleprikon
Copy link
Contributor

No description provided.

@litleleprikon litleleprikon force-pushed the feature/428-improve-advanced-mode branch 2 times, most recently from 3966397 to 11473ec Compare December 10, 2019 15:41
@beevee beevee requested a review from a team December 11, 2019 09:06
@litleleprikon litleleprikon force-pushed the feature/428-improve-advanced-mode branch 2 times, most recently from 9f4efbf to b3eb635 Compare December 11, 2019 10:00
@litleleprikon litleleprikon force-pushed the feature/428-improve-advanced-mode branch 3 times, most recently from 8c277c1 to ac017d5 Compare December 22, 2019 19:20
@litleleprikon litleleprikon force-pushed the feature/428-improve-advanced-mode branch 2 times, most recently from 50fec12 to 5c570e5 Compare December 26, 2019 14:37
@coveralls
Copy link

coveralls commented Dec 26, 2019 •
edited
Loading

Coverage Status

Coverage decreased (-0.3%) to 80.139% when pulling db4e594 on feature/428-improve-advanced-mode into c447ce4 on master.

@litleleprikon litleleprikon force-pushed the feature/428-improve-advanced-mode branch 2 times, most recently from eea1557 to 55b77db Compare January 4, 2020 00:04
@litleleprikon litleleprikon removed the WIP Work in progress label Jan 4, 2020
@litleleprikon litleleprikon force-pushed the feature/428-improve-advanced-mode branch 2 times, most recently from 361e158 to 9c4ca7f Compare January 10, 2020 15:13
borovskyav
borovskyav suggested changes Jan 12, 2020
View reviewed changes
metric_source/fetched_metrics.go Outdated Show resolved Hide resolved
metric_source/fetched_metrics.go Outdated Show resolved Hide resolved
metric_source/remote/response.go Show resolved Hide resolved
metric_source/fetched_metrics.go Outdated Show resolved Hide resolved
metric_source/trigger_metrics.go Outdated Show resolved Hide resolved
checker/check.go Outdated Show resolved Hide resolved
checker/check.go Outdated Show resolved Hide resolved
checker/check.go Outdated Show resolved Hide resolved
metric_source/source.go Show resolved Hide resolved
database/redis/compatibility26.go Outdated Show resolved Hide resolved
litleleprikon
litleleprikon commented Jan 29, 2020
View reviewed changes
docker-compose.yml Outdated Show resolved Hide resolved
litleleprikon
litleleprikon commented Feb 13, 2020
View reviewed changes
checker/check.go Outdated Show resolved Hide resolved
@litleleprikon litleleprikon force-pushed the feature/428-improve-advanced-mode branch 2 times, most recently from ee5e240 to 06cb1c5 Compare March 10, 2020 13:04
@litleleprikon litleleprikon force-pushed the feature/428-improve-advanced-mode branch 2 times, most recently from 97f20fb to d1ef079 Compare April 6, 2020 21:03
borovskyav
borovskyav suggested changes May 17, 2020
View reviewed changes
Copy link
Contributor

@borovskyav borovskyav left a comment •
edited
Loading

Choose a reason for hiding this comment

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

В общем ничего серьезного, всё красиво, только давай структурно подойдем к новой жизни с массивом картинок в нотификации.
А и еще тебе удобно жить в мире, когда ты сам резолвишь мои комменты?) Ну и наоборот, когда ты ревьювер и резолвят твои комменты) Я просто привык жить в мире когда мне пишут замечание, я правлю, пишу что-то типа "done", а там уже либо резолв либо обсуждение. Я конечно не настаиваю, может я ничего не понимаю и возможно жить и так)

senders/webhook/payload.go Show resolved Hide resolved
senders/victorops/send.go Show resolved Hide resolved
senders/telegram/send.go Show resolved Hide resolved
senders/pushover/pushover.go Show resolved Hide resolved
senders/pagerduty/send.go Show resolved Hide resolved
senders/opsgenie/send.go Show resolved Hide resolved
refactor: Rename MakeEmptyMetricsData
2a12879 
Rename MakeEmptyMetricsData as in go default name for constructors
should be with schema like NewStructName and name of this function is
not in go way.

Reference to Effective go:

> Similarly, the function to make new instances of ring.Ring—which is
the definition of a constructor in Go—would normally be called NewRing,
but since Ring is the only type exported by the package, and since the
package is called ring, it's called just New, which clients of the
package see as ring.New. Use the package structure to help you choose
good names.
tests(checker): Add checker tests and benchmarks
7d6311a 
Add checker tests before improvement of advanced mode and benchmark to
compare performance before and after improvements.

Relates #428
@litleleprikon litleleprikon force-pushed the feature/428-improve-advanced-mode branch from d1ef079 to d3be627 Compare May 27, 2020 18:55
@borovskyav
Copy link
Contributor

Давай не забудем упомянуть про поломку совместимости вебхуков в ченжлоге еще, чтобы красиво всё было)

@borovskyav
Copy link
Contributor

И давай сразу как вольём задачу в мастер, сделаем ишью о добавлении мультикартиночности)

borovskyav
borovskyav previously approved these changes May 28, 2020
View reviewed changes
feat: Improve advanced mode
db4e594 
Datatype that stores metrics for check is changed to multiple datatypes
that handle different operations on data. Fetcher interface is changed to
return new implemented datatypes. Checker is changed to handle triggers
with multiple targets in new way. API, notifier are changed to fit new
CheckData struct.

Relates #428
@litleleprikon litleleprikon force-pushed the feature/428-improve-advanced-mode branch from d3be627 to db4e594 Compare May 28, 2020 10:44
@litleleprikon litleleprikon merged commit 326eb0d into master May 28, 2020
@litleleprikon litleleprikon deleted the feature/428-improve-advanced-mode branch May 28, 2020 10:59
@borovskyav borovskyav mentioned this pull request Jun 9, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@borovskyav borovskyav borovskyav approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@litleleprikon @coveralls @borovskyav