Skip to content

Conversation

@ecrupper
Copy link
Contributor

Part of the effort of go-vela/community#159

PR accomplishes the following:

  • Set event action field in scm/github/webhook.go if the event is pull_request or comment. Otherwise, this field remains nil.
  • Use the event action field, if not nil, to create a scoped <event>:<action> combination to utilize for determining whether or not a step should execute. There will be a separate PR to address this at runtime in the worker code.
  • Adds the event_action field to the ddl

@ecrupper ecrupper requested a review from a team as a code owner April 26, 2022 21:09
@ecrupper ecrupper self-assigned this Apr 26, 2022
@codecov
Copy link

codecov bot commented Apr 26, 2022

Codecov Report

Merging #630 (d11a9ec) into master (8d3befc) will increase coverage by 0.00%.
The diff coverage is 75.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #630   +/-   ##
=======================================
  Coverage   55.47%   55.48%           
=======================================
  Files         195      195           
  Lines       15740    15749    +9     
=======================================
+ Hits         8732     8738    +6     
- Misses       6644     6646    +2     
- Partials      364      365    +1     
Impacted Files Coverage Δ
compiler/native/compile.go 61.47% <62.50%> (-0.18%) ⬇️
scm/github/webhook.go 90.19% <100.00%> (+0.08%) ⬆️

Copy link
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

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

I have a question about restricting the PR edited event. Otherwise this looks good

Comment on lines 187 to 193
// if the edits to the PR are not related to the title or base branch, ignore them
if strings.EqualFold(payload.GetAction(), "edited") {
title := payload.GetChanges().GetTitle()
base := payload.GetChanges().GetBase()

if title == nil && base == nil {
return &types.Webhook{Hook: h}, nil
Copy link
Member

Choose a reason for hiding this comment

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

Why? What if someone had a workflow that validated the PR description followed some format, would we not support that?

cognifloyd
cognifloyd previously approved these changes Apr 27, 2022
@wass3r
Copy link
Collaborator

wass3r commented Apr 29, 2022

conflicts :(

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

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.

5 participants