Skip to content

Conversation

@ecrupper
Copy link
Contributor

Closes go-vela/community#657

Trying to add some extra context during the webhook processing without overlapping some of the normal debug logging

@ecrupper ecrupper requested a review from a team as a code owner September 16, 2022 18:23
@ecrupper ecrupper self-assigned this Sep 16, 2022
@codecov
Copy link

codecov bot commented Sep 16, 2022

Codecov Report

Merging #697 (30479d5) into main (f60ee1d) will decrease coverage by 0.06%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #697      +/-   ##
==========================================
- Coverage   55.05%   54.98%   -0.07%     
==========================================
  Files         211      211              
  Lines       15994    16014      +20     
==========================================
  Hits         8805     8805              
- Misses       6811     6831      +20     
  Partials      378      378              
Impacted Files Coverage Δ
api/webhook.go 0.00% <0.00%> (ø)
scm/github/repo.go 74.55% <0.00%> (-0.23%) ⬇️

api/webhook.go Outdated

h, r, b := webhook.Hook, webhook.Repo, webhook.Build

logrus.Debugf("Hook generated from SCM: %v", h)
Copy link
Contributor

Choose a reason for hiding this comment

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

how do these structs look when printed out? are they easy enough to read?

Copy link
Contributor

Choose a reason for hiding this comment

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

also leaving this here:
https://github.com/go-vela/server/blob/main/api/user.go#L75-L77

you might be able to use fields

plyr4
plyr4 previously approved these changes Sep 26, 2022
Copy link
Contributor

@plyr4 plyr4 left a comment

Choose a reason for hiding this comment

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

lgtm ty

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

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.

Improve logging inside webhook

5 participants