Skip to content
Prev Previous commit
Next Next commit
Merge branch 'main' into 16457_wiki_webhook
  • Loading branch information
f0086 authored Aug 30, 2022
commit 0442361b6871fd1fe02e797f967fce8721d3431d
6 changes: 3 additions & 3 deletions modules/notification/base/notifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ type Notifier interface {
NotifyNewWikiPage(doer *user_model.User, repo *repo_model.Repository, page, comment string)
NotifyEditWikiPage(doer *user_model.User, repo *repo_model.Repository, page, comment string)
Copy link
Member

Choose a reason for hiding this comment

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

Update is a better word than Edit here imo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also thought about that when naming it. There is an "UpdateRelease" and an "UpdateComment", so It would be consistent with that. But this is the only place where I find the word "Update", there are other "synonyms" in use like "Change", which makes semantically more sense in that cases. So I don't "break the rule" in naming the notifier for editing a wiki page EditWikiPage to keep it semantically consistent. Take a look at all the notifiers (here). The names follow semantic names instead of consistent "CRUD"-like names. If EditWikiPage should be renamed to UpdateWikiPage, I also need to rename NewWikiPage to CreateWikiPage to bring it in line with some other actions. What do you think @lunny?

NotifyDeleteWikiPage(doer *user_model.User, repo *repo_model.Repository, page string)
NotifyNewRelease(rel *models.Release)
NotifyUpdateRelease(doer *user_model.User, rel *models.Release)
NotifyDeleteRelease(doer *user_model.User, rel *models.Release)
NotifyNewRelease(rel *repo_model.Release)
NotifyUpdateRelease(doer *user_model.User, rel *repo_model.Release)
NotifyDeleteRelease(doer *user_model.User, rel *repo_model.Release)
NotifyPushCommits(pusher *user_model.User, repo *repo_model.Repository, opts *repository.PushUpdateOptions, commits *repository.PushCommits)
NotifyCreateRef(doer *user_model.User, repo *repo_model.Repository, refType, refFullName, refID string)
NotifyDeleteRef(doer *user_model.User, repo *repo_model.Repository, refType, refFullName string)
Expand Down
You are viewing a condensed version of this merge commit. You can view the full changes here.