From 66f45c2bf63ca6e2ebab60512048190f86571b5a Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Fri, 21 Mar 2025 00:43:47 +0000 Subject: [PATCH] refactor(fun/clbot): drop changeShouldBeSkipped functionality Change-Id: Id0b05b070dfdb8099c6c32761295dcb17db1aea4 Reviewed-on: https://cl.snix.dev/c/snix/+/30224 Tested-by: besadii Autosubmit: Florian Klink Reviewed-by: Ilan Joselevich --- fun/clbot/clbot.go | 22 +++------------------- fun/clbot/clbot_test.go | 24 ------------------------ fun/clbot/default.nix | 1 - 3 files changed, 3 insertions(+), 44 deletions(-) delete mode 100644 fun/clbot/clbot_test.go diff --git a/fun/clbot/clbot.go b/fun/clbot/clbot.go index 9f3b29fdb..82e2c4ea1 100644 --- a/fun/clbot/clbot.go +++ b/fun/clbot/clbot.go @@ -40,8 +40,7 @@ var ( notifyRepo = flag.String("notify_repo", "depot", "Repo name to notify about") notifyBranches = stringSetFlag{} - neverPing = flag.String("never_ping", "marcus", "Comma-separated terms that should never ping users") - onlyDisplay = flag.String("only_display", "", "Comma-separated substrings of the gerrit CL Change Subject that should be shown (everything else is dropped)") + neverPing = flag.String("never_ping", "marcus", "Comma-separated terms that should never ping users") ) func init() { @@ -193,21 +192,6 @@ func nopingAll(username, message string) string { return strings.ReplaceAll(message, username, noping(username)) } -// changeShouldBeSkipped applies the list of channels in `onlyDisplay` -// to whether we should skip displaying a CL. -func changeShouldBeSkipped(onlyDisplay string, changeSubject string) bool { - // case when we don’t want to filter - if onlyDisplay == "" { - return false - } - for _, needle := range strings.Split(onlyDisplay, ",") { - if strings.Contains(changeSubject, needle) { - return false - } - } - return true -} - func patchSetURL(c gerritevents.Change, p gerritevents.PatchSet) string { return fmt.Sprintf("https://cl.snix.dev/%d", c.Number) } @@ -263,13 +247,13 @@ func main() { var parsedMsg string switch e := e.(type) { case *gerritevents.PatchSetCreated: - if e.Change.Project != *notifyRepo || !notifyBranches[e.Change.Branch] || e.PatchSet.Number != 1 || changeShouldBeSkipped(*onlyDisplay, e.Change.Subject) { + if e.Change.Project != *notifyRepo || !notifyBranches[e.Change.Branch] || e.PatchSet.Number != 1 { continue } user := username(e.PatchSet.Uploader) parsedMsg = nopingAll(user, fmt.Sprintf("CL/%d proposed by %s - %s - %s", e.Change.Number, user, e.Change.Subject, patchSetURL(e.Change, e.PatchSet))) case *gerritevents.ChangeMerged: - if e.Change.Project != *notifyRepo || !notifyBranches[e.Change.Branch] || changeShouldBeSkipped(*onlyDisplay, e.Change.Subject) { + if e.Change.Project != *notifyRepo || !notifyBranches[e.Change.Branch] { continue } owner := username(e.Change.Owner) diff --git a/fun/clbot/clbot_test.go b/fun/clbot/clbot_test.go deleted file mode 100644 index 567540c36..000000000 --- a/fun/clbot/clbot_test.go +++ /dev/null @@ -1,24 +0,0 @@ -package main - -import ( - "testing" -) - -func TestChangeShouldBeSkipped(t *testing.T) { - dontSkipAny := "" - if changeShouldBeSkipped(dontSkipAny, "mysubject") { - t.Fatal("dontSkipAny should not not be skip any") - } - - showThese := "A,B" - if changeShouldBeSkipped(showThese, "A") { - t.Fatal("A should be shown") - } - if changeShouldBeSkipped(showThese, "B") { - t.Fatal("B should be shown") - } - if !changeShouldBeSkipped(showThese, "C") { - t.Fatal("C should not be shown") - } - -} diff --git a/fun/clbot/default.nix b/fun/clbot/default.nix index 8ba6db8d3..ebfe7ce7f 100644 --- a/fun/clbot/default.nix +++ b/fun/clbot/default.nix @@ -10,7 +10,6 @@ pkgs.buildGoModule { root = ./.; fileset = lib.fileset.unions [ ./clbot.go - ./clbot_test.go ./go.mod ./go.sum ./backoffutil