Codecov Report

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

@@            Coverage Diff            @@
##             master    #4299   +/-   ##
=========================================
  Coverage          ?   38.86%           
=========================================
  Files             ?      365           
  Lines             ?    51341           
  Branches          ?        0           
=========================================
  Hits              ?    19953           
  Misses            ?    28516           
  Partials          ?     2872

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 cac9e6e...32b3e68. Read the comment docs.

It's been a while. Should I merge master into this branch now to make review easier, or is it best to wait?

I'm confused about why drone failed. Is it a drone bug, or something I did wrong?

@aunger let me re-trigger drone.

@aunger drone is all good now. I suggest asking in the #develop channel on discord for a maintainer to review this PR

Thanks for your help, @techknowlogick, but I've tried Discord a few times and don't get much response. Would you mind doing the review yourself?

@lunny, would you review this too? I see you hearted it. It's been languishing for two months now.

Thanks.

@aunger I will review and test it. but that maybe spend my some time.

LGTM

@ethantkoenig Mind reviewing this PR? This would turn one of your ufw customizations into an admin setting. https://github.com/unfoldingWord-dev/gogs/blob/a2489be2c7ab4d3d20838135bb1d538c2eb5f40c/models/repo.go#L1363

@aunger I won't be able to, sorry.

Can default webhooks be stored to new table, so that this functionality could be later extended more easily?

@lafriks Someone could certainly implement it that way. This PR's KISS approach is better in some ways though, because the plumbing is more easily shared (don't need an abstraction of a webhook if they are all the same) and it requires less attention to table schemas. What future extensions do you have in mind?

@aunger I would like to have ability to have forced webhooks for all repositories, updating webhooks for repositories when changing default webhook, ability to select/deselect from default webhooks in repository settings etc

@lafriks Great ideas. Site-wide hooks are more flexible. I was weighing most of that before I implemented this, but I didn't get any feedback (#770 (comment)), so I decided to go simple ("agile"?) with this implementation, because it's all my organization needs right now (and because it's what #770 explicitly asked for).

@lafriks I won't tackle site-wide hooks right now. If you're going to code it up now, that's awesome. If not, I hope you consider merging this implementation in the meantime. Thanks.