Skip to content

Conversation

@ecrupper
Copy link
Contributor

Sometimes the webhook delivery will no longer be available due to how old it is, not whether it is in the 100 most recent. Upon failure, it's a better idea to give a general error to avoid confusing the user.

@ecrupper ecrupper requested a review from a team as a code owner September 29, 2022 19:05
@ecrupper ecrupper self-assigned this Sep 29, 2022
Copy link
Contributor

@KellyMerrick KellyMerrick left a comment

Choose a reason for hiding this comment

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

lgtm

@codecov
Copy link

codecov bot commented Sep 29, 2022

Codecov Report

Merging #707 (faeb743) into main (8e8d0d4) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #707   +/-   ##
=======================================
  Coverage   54.98%   54.98%           
=======================================
  Files         211      211           
  Lines       16014    16014           
=======================================
  Hits         8805     8805           
  Misses       6831     6831           
  Partials      378      378           
Impacted Files Coverage Δ
scm/github/webhook.go 86.06% <0.00%> (ø)

Copy link
Contributor

@jbrockopp jbrockopp left a comment

Choose a reason for hiding this comment

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

LGTM

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.

6 participants