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.

Sign up for GitHub

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

Jump to bottom

Refactor push update #13381

Merged
merged 5 commits into from
Dec 8, 2020
Merged

Refactor push update #13381

merged 5 commits into from
Dec 8, 2020

Conversation

lunny
Copy link
Member

@lunny lunny commented Nov 1, 2020 •
edited
Loading

This PR refactor the push update logic to make it more clear and easier to change. Replace #10426

@lunny lunny added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Nov 1, 2020
@lunny lunny mentioned this pull request Nov 1, 2020
Closed
@codecov-io
Copy link

codecov-io commented Nov 1, 2020 •
edited
Loading

Codecov Report

Merging #13381 (09fd01f) into master (682f0b0) will decrease coverage by 0.02%.
The diff coverage is 61.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13381      +/-   ##
==========================================
- Coverage   42.25%   42.23%   -0.03%     
==========================================
  Files         708      708              
  Lines       77191    77192       +1     
==========================================
- Hits        32620    32603      -17     
- Misses      39211    39216       +5     
- Partials     5360     5373      +13
Impacted Files Coverage Δ
services/repository/push.go 44.64% <44.00%> (-13.39%) ⬇️
modules/notification/action/action.go 60.49% <70.58%> (+2.23%) ⬆️
modules/repository/push.go 47.61% <0.00%> (-4.77%) ⬇️
services/pull/check.go 48.90% <0.00%> (-2.92%) ⬇️
modules/notification/ui/ui.go 93.05% <0.00%> (-2.78%) ⬇️
modules/repository/commits.go 84.09% <0.00%> (-2.28%) ⬇️
models/notification.go 66.25% <0.00%> (-0.99%) ⬇️
services/pull/pull.go 40.68% <0.00%> (-0.50%) ⬇️
models/error.go 38.66% <0.00%> (-0.33%) ⬇️
modules/notification/mail/mail.go 39.08% <0.00%> (ø)
... and 4 more

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 682f0b0...09fd01f. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 1, 2020
lafriks
lafriks reviewed Nov 1, 2020
View reviewed changes
services/repository/push.go Outdated Show resolved Hide resolved
@6543 6543 mentioned this pull request Nov 9, 2020
Closed
6 tasks
@lunny
Copy link
Member Author

lunny commented Dec 5, 2020

@lafriks 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 Dec 5, 2020
@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 Dec 8, 2020
@6543 6543 merged commit ab22ab4 into go-gitea:master Dec 8, 2020
@lunny lunny deleted the lunny/refactor_push_update branch December 8, 2020 03:31
@zeripath zeripath mentioned this pull request Dec 11, 2020
Closed
6 tasks
zeripath added a commit to zeripath/gitea that referenced this pull request Dec 11, 2020
@zeripath
Ensure that pusher is not nil preventing panic in push
322b561 
PR go-gitea#13381 refactored notification actions to call NotifyCreateRef and
NotifyDeleteRef in a slightly different manner. This leads to the pusher
not being set before the call.

This PR ensures that the pusher is updated before the call.

Fix go-gitea#13940

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request Dec 11, 2020
Merged
6543 pushed a commit that referenced this pull request Dec 11, 2020
@zeripath
Ensure that pusher is not nil preventing panic in push (#13945)
f2f99a7 
PR #13381 refactored notification actions to call NotifyCreateRef and
NotifyDeleteRef in a slightly different manner. This leads to the pusher
not being set before the call.

This PR ensures that the pusher is updated before the call.

Fix #13940

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added this to the 1.14.0 milestone Dec 14, 2020
@CirnoT CirnoT mentioned this pull request Dec 15, 2020
Merged
@go-gitea go-gitea locked and limited conversation to collaborators Jan 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@6543 6543 6543 approved these changes

@lafriks lafriks lafriks approved these changes

Assignees
No one assigned
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
Milestone
1.14.0
Development

Successfully merging this pull request may close these issues.
6 participants
@lunny @codecov-io @lafriks @6543 @zeripath @GiteaBot