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

feat: adding a delete_event #340

Merged
merged 18 commits into from
Jan 11, 2024
Merged

feat: adding a delete_event #340

merged 18 commits into from
Jan 11, 2024

Conversation

claire1618
Copy link
Contributor

This code enables the delete webhook in Vela, so users can select to trigger a build when a branch or tag are deleted.

@claire1618 claire1618 requested a review from a team as a code owner December 29, 2023 21:52
library/actions/delete.go Show resolved Hide resolved
library/actions/delete_test.go Outdated Show resolved Hide resolved
library/events_test.go Outdated Show resolved Hide resolved
library/repo.go Outdated Show resolved Hide resolved
Copy link

codecov bot commented Dec 29, 2023

Codecov Report

Attention: 29 lines in your changes are missing coverage. Please review.

Comparison is base (a91bd54) 96.18% compared to head (5198a27) 95.85%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #340      +/-   ##
==========================================
- Coverage   96.18%   95.85%   -0.33%     
==========================================
  Files          69       70       +1     
  Lines        7417     7504      +87     
==========================================
+ Hits         7134     7193      +59     
- Misses        205      233      +28     
  Partials       78       78              
Files Coverage Δ
library/actions/comment.go 92.30% <ø> (ø)
library/actions/delete.go 100.00% <100.00%> (ø)
library/actions/push.go 100.00% <ø> (ø)
library/events.go 82.58% <95.23%> (+2.58%) ⬆️
library/repo.go 93.59% <3.44%> (-6.41%) ⬇️

Copy link
Contributor

@ecrupper ecrupper left a comment

Choose a reason for hiding this comment

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

There shouldn't be a new field in library.Repo or database.Repo — only in library.Events should there be a new action set for delete events.

database/repo.go Outdated Show resolved Hide resolved
item_test.go Outdated Show resolved Hide resolved
library/repo.go Outdated Show resolved Hide resolved
library/repo.go Outdated Show resolved Hide resolved
library/repo.go Outdated Show resolved Hide resolved
library/secret.go Outdated Show resolved Hide resolved
database/repo_test.go Outdated Show resolved Hide resolved
database/repo_test.go Outdated Show resolved Hide resolved
Copy link
Member

@wass3rw3rk wass3rw3rk left a comment

Choose a reason for hiding this comment

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

lgtm, do we want to add delete here as well:

types/yaml/ruleset.go

Lines 147 to 163 in 26e54c8

for _, e := range rules.Event {
switch e {
// backwards compatibility
// pull_request = pull_request:opened + pull_request:synchronize + pull_request:reopened
// comment = comment:created + comment:edited
case constants.EventPull:
events = append(events,
constants.EventPull+":"+constants.ActionOpened,
constants.EventPull+":"+constants.ActionSynchronize,
constants.EventPull+":"+constants.ActionReopened)
case constants.EventComment:
events = append(events,
constants.EventComment+":"+constants.ActionCreated,
constants.EventComment+":"+constants.ActionEdited)
default:
events = append(events, e)
}
, so when you specify just delete it will add both actions for you? not saying we have to, just asking to get thoughts.

library/events.go Show resolved Hide resolved
library/events.go Show resolved Hide resolved
library/events.go Show resolved Hide resolved
library/events.go Show resolved Hide resolved
library/events.go Show resolved Hide resolved
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

@claire1618 claire1618 merged commit 0d0e223 into main Jan 11, 2024
10 checks passed
@claire1618 claire1618 deleted the feat/deleteEvent2 branch January 11, 2024 19:21
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.

4 participants