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: Update notification engine #9386

Merged
merged 3 commits into from
May 23, 2022

Conversation

todaywasawesome
Copy link
Contributor

Signed-off-by: todaywasawesome dan@codefresh.io

Notification Engine is several months behind and includes several bug fixes and two additional features.

@crenshaw-dev @alexmt There is an open question if we should cherrypick this into 2.4. Given how far behind the current package is, my instinct is that we should include it. Argo Rollouts is using a version from Jan '22 while CD is using one from Nov '21.

CC @kevinchen-verkada who is advocating for these changes.

Signed-off-by: todaywasawesome <dan@codefresh.io>
@codecov
Copy link

codecov bot commented May 12, 2022

Codecov Report

Merging #9386 (b3a2856) into master (d0fe8f1) will decrease coverage by 0.50%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #9386      +/-   ##
==========================================
- Coverage   46.22%   45.71%   -0.51%     
==========================================
  Files         218      221       +3     
  Lines       25914    26289     +375     
==========================================
+ Hits        11978    12019      +41     
- Misses      12278    12610     +332     
- Partials     1658     1660       +2     
Impacted Files Coverage Δ
server/badge/badge.go 78.72% <0.00%> (-5.57%) ⬇️
applicationset/generators/pull_request.go 39.13% <0.00%> (-3.73%) ⬇️
util/session/sessionmanager.go 67.43% <0.00%> (-1.32%) ⬇️
util/cert/cert.go 81.81% <0.00%> (-0.59%) ⬇️
server/application/application.go 31.40% <0.00%> (-0.43%) ⬇️
applicationset/services/pull_request/gitlab.go 85.29% <0.00%> (ø)
pkg/apiclient/grpcproxy.go 0.00% <0.00%> (ø)
pkg/apiclient/apiclient.go 0.81% <0.00%> (ø)
reposerver/repository/repository.go 60.04% <0.00%> (+0.34%) ⬆️
util/cmp/stream.go 52.59% <0.00%> (+0.44%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d0fe8f1...b3a2856. Read the comment docs.

@crenshaw-dev
Copy link
Member

@todaywasawesome I'm not opposed! Looks like it needs a make notification-docs.

Signed-off-by: todaywasawesome <dan@codefresh.io>
Signed-off-by: todaywasawesome <dan@codefresh.io>
Copy link
Collaborator

@alexmt alexmt left a comment

Choose a reason for hiding this comment

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

Reviewed changes in notification-engine one more time .

Looks like PR would pull in several bug fixes and new features that would be good to test. So I think it is good idea to bump notification engine version even though we are very close to the release.

@alexmt alexmt merged commit f788c4e into argoproj:master May 23, 2022
alexmt pushed a commit that referenced this pull request May 24, 2022
refactor: Update notification engine  (#9386)

Signed-off-by: todaywasawesome <dan@codefresh.io>
@todaywasawesome todaywasawesome deleted the update-notification-engine branch November 2, 2022 21:32
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.

None yet

3 participants