Skip to content

Conversation

@lunny
Copy link
Member

@lunny lunny commented Nov 4, 2019

As title.

@lunny lunny added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Nov 4, 2019
@6543
Copy link
Member

6543 commented Nov 4, 2019

@lunny

mode, _ := models.AccessLevel(doer, rel.Repo)
	if err := webhook.PrepareWebhooks(rel.Repo, models.HookEventRelease, &api.ReleasePayload{
		Action:     api.HookReleaseUpdated,
		Release:    rel.APIFormat(),
		Repository: rel.Repo.APIFormat(mode),
		Sender:     doer.APIFormat(),
	}); err != nil {
		log.Error("PrepareWebhooks: %v", err)
	}

exists 3 times only differ in Action: xy

does it make sence to move this in a seperate function wich is used by all three funcitons?
or will it block later extentions?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 4, 2019
@lunny lunny force-pushed the lunny/move_release_webhook branch from b575087 to f4101fd Compare November 5, 2019 01:36
@lunny
Copy link
Member Author

lunny commented Nov 5, 2019

@6543 done.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 5, 2019
@6543
Copy link
Member

6543 commented Nov 5, 2019

CI.restart()

@techknowlogick
Copy link
Member

restarted

@lunny lunny force-pushed the lunny/move_release_webhook branch from f4101fd to 64761a5 Compare November 5, 2019 06:57
@codecov-io
Copy link

codecov-io commented Nov 5, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@6d42add). Click here to learn what that means.
The diff coverage is 56%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #8817   +/-   ##
=========================================
  Coverage          ?   41.26%           
=========================================
  Files             ?      543           
  Lines             ?    69769           
  Branches          ?        0           
=========================================
  Hits              ?    28792           
  Misses            ?    37298           
  Partials          ?     3679
Impacted Files Coverage Δ
modules/notification/webhook/webhook.go 40.18% <54.54%> (ø)
services/release/release.go 24.77% <66.66%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d42add...0c787e0. Read the comment docs.

@6543
Copy link
Member

6543 commented Nov 5, 2019

@techknowlogick can you reviwe&merge?

@lunny lunny force-pushed the lunny/move_release_webhook branch from 64761a5 to 72901da Compare November 5, 2019 11:11
@lunny lunny force-pushed the lunny/move_release_webhook branch from 72901da to 738a0e0 Compare November 6, 2019 06:51
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Nov 6, 2019
@lunny lunny merged commit 0109229 into go-gitea:master Nov 6, 2019
@lunny lunny deleted the lunny/move_release_webhook branch November 6, 2019 08:25
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/refactoring Existing code has been cleaned up. There should be no new functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants