-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Webhooks for repo creation/deletion #1663
Conversation
<div class="seven wide column"> | ||
<div class="field"> | ||
<div class="ui checkbox"> | ||
<input class="hidden" name="repository" type="checkbox" tabindex="0" {{if .Webhook.PullRequest}}checked{{end}}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.Webhook.Repository
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
20dc0d1
to
f2a82a8
Compare
40e87dc
to
989ca37
Compare
Rebased to resolve conflicts. |
0bfaa20
to
66eebe8
Compare
66eebe8
to
1c40c5e
Compare
1c40c5e
to
37eea63
Compare
37eea63
to
e7525e4
Compare
Will be great to see this merged so I can add support to the https://github.com/jenkinsci/gitea-plugin |
models/repo.go
Outdated
Organization: u.APIFormat(), | ||
Sender: doer.APIFormat(), | ||
}); err != nil { | ||
return fmt.Errorf("PrepareWebhooks: %v", err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not go HookQueue.Add(repo.ID)
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
models/repo.go
Outdated
@@ -1246,6 +1246,13 @@ func createRepository(e *xorm.Session, u *User, repo *Repository) (err error) { | |||
return fmt.Errorf("getOwnerTeam: %v", err) | |||
} else if err = t.addRepository(e, repo); err != nil { | |||
return fmt.Errorf("addRepository: %v", err) | |||
} else if err = PrepareWebhooks(repo, HookEventRepository, &api.RepositoryPayload{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will result database table lock
on sqlite since we have a transaction here and another in PrepareWebhooks
. It seems you have to create a prepareWebhooks(e Engine...
method to avoid that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
e7525e4
to
5095cce
Compare
@ethantkoenig Still fail in sqlite on function |
@lunny Good catch, fixed |
@ethantkoenig but maybe you forgot to replace |
return prepareWebhook(x, w, repo, event, p) | ||
} | ||
|
||
func prepareWebhook(e Engine, w *Webhook, repo *Repository, event HookEventType, p api.Payloader) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing HookEventRepository
on switch case. And missing handle on GetSlackPayload
and GetDiscordPayload
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added missing case to HookEventRepository
. Will hopefully get change to add handles to GetSlackPayload
and GetDiscordPayload
tomorrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lunny Done
LGTM |
LGTM |
(party) - when can we expect a release? |
This is a part of v1.3 should be released 2017.10.25 |
@ethantkoenig maybe add |
And the option should be only when an orgnization webhook, when create/edit a repository webhoo, it should be hidden.
|
Allow organizations to have webhooks that are triggered by repository creation/deletion. Partially addresses #2113.
Depends on go-gitea/go-sdk#55(merged)