From 3205ee6571a7189c2390939a94f096bf083e0d51 Mon Sep 17 00:00:00 2001 From: Haliq Date: Fri, 24 Jul 2020 17:04:46 +0800 Subject: [PATCH 01/39] create new variant for config status rule states extending the enumeration without modifying the domain for github status_state --- lib/action.ml | 14 +++++++++----- lib/config.ml | 17 ++++++++++++----- lib/notabot.atd | 2 ++ test/notabot.json | 9 +++++---- 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index bd85df92..f93b386f 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -156,16 +156,20 @@ let partition_commit cfg files = List.dedup_and_sort ~compare:String.compare (List.concat channels) let hide_cancelled (notification : status_notification) cfg = - let is_cancelled_status = + match List.exists cfg.status_rules.status ~f:(Poly.equal Cancelled) with + | true -> false + | false -> let { state; description; _ } = notification in let r = Re.Str.regexp_case_fold "^\\(Build #[0-9]+ canceled by .+\\|Failed (exit status 255)\\)$" in - match description, state with + ( match description, state with | Some s, Failure when Re.Str.string_match r s 0 -> true | _ -> false - in - is_cancelled_status && cfg.suppress_cancelled_events + ) let hide_success (n : status_notification) (ctx : Context.t) = + match List.exists ctx.cfg.status_rules.status ~f:(Poly.equal ConsecutiveSuccess) with + | true -> false + | false -> match n.state with | Success -> List.exists @@ -193,7 +197,7 @@ let partition_status (ctx : Context.t) (n : status_notification) = | false -> Lwt.return (partition_commit cfg commit.files) in let res = - match List.exists cfg.status_rules.status ~f:(Poly.equal n.state) with + match List.exists cfg.status_rules.status ~f:(Poly.equal (State n.state)) with | false -> Lwt.return [] | true -> match List.exists ~f:id [ hide_cancelled n cfg; hide_success n ctx ] with diff --git a/lib/config.ml b/lib/config.ml index ef7217f4..231bb2b0 100644 --- a/lib/config.ml +++ b/lib/config.ml @@ -1,9 +1,14 @@ open Devkit module Chan_map = Map.Make (String) +type config_status_state = + | State of Github_t.status_state + | Cancelled + | ConsecutiveSuccess + type status_rules = { title : string list option; - status : Github_t.status_state list; + status : config_status_state list; } type t = { @@ -65,10 +70,12 @@ let make (json_config : Notabot_t.config) (secrets : Notabot_t.secrets) = let j = json_config.status_rules.status in List.filter_map id [ - (if j.success then Some Success else None); - (if j.failure then Some Failure else None); - (if j.pending then Some Pending else None); - (if j.error then Some Error else None); + (if j.success then Some (State Success) else None); + (if j.failure then Some (State Failure) else None); + (if j.pending then Some (State Pending) else None); + (if j.error then Some (State Error) else None); + (if j.cancelled then Some Cancelled else None); + (if j.consecutive_success then Some ConsecutiveSuccess else None); ] in { title = json_config.status_rules.title; status } diff --git a/lib/notabot.atd b/lib/notabot.atd index a457263e..334cc9a9 100644 --- a/lib/notabot.atd +++ b/lib/notabot.atd @@ -56,6 +56,8 @@ type status_state = { failure: bool; pending: bool; error: bool; + cancelled: bool; + consecutive_success: bool; } type status_config = { diff --git a/test/notabot.json b/test/notabot.json index f11e940e..fc41fe06 100644 --- a/test/notabot.json +++ b/test/notabot.json @@ -10,7 +10,9 @@ "pending": false, "success": true, "failure": true, - "error": true + "error": true, + "cancelled": false, + "consecutive_success": false } }, "prefix_rules": { @@ -72,6 +74,5 @@ "chan": "frontend-bot" } ] - }, - "suppress_cancelled_events": true -} + } +} \ No newline at end of file From df473bc477714cbe134ec92b74959669ece77886 Mon Sep 17 00:00:00 2001 From: Haliq Date: Mon, 27 Jul 2020 15:11:18 +0800 Subject: [PATCH 02/39] cancelled status rule now accepts a string as regexp to match description --- lib/action.ml | 17 +++++++++++++---- lib/config.ml | 10 +++++----- lib/notabot.atd | 3 +-- test/notabot.json | 2 +- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index f93b386f..6dd03d0e 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -156,11 +156,20 @@ let partition_commit cfg files = List.dedup_and_sort ~compare:String.compare (List.concat channels) let hide_cancelled (notification : status_notification) cfg = - match List.exists cfg.status_rules.status ~f:(Poly.equal Cancelled) with - | true -> false - | false -> + let f a x = + match a with + | Some _ as v -> v + | None -> + match x with + | Cancelled r -> Some r + | _ -> None + in + let regexp_opt = List.fold cfg.status_rules.status ~init:None ~f in + match regexp_opt with + | None -> false + | Some regexp -> let { state; description; _ } = notification in - let r = Re.Str.regexp_case_fold "^\\(Build #[0-9]+ canceled by .+\\|Failed (exit status 255)\\)$" in + let r = Re.Str.regexp_case_fold regexp in ( match description, state with | Some s, Failure when Re.Str.string_match r s 0 -> true | _ -> false diff --git a/lib/config.ml b/lib/config.ml index 231bb2b0..245ffbb1 100644 --- a/lib/config.ml +++ b/lib/config.ml @@ -3,7 +3,7 @@ module Chan_map = Map.Make (String) type config_status_state = | State of Github_t.status_state - | Cancelled + | Cancelled of string | ConsecutiveSuccess type status_rules = { @@ -20,7 +20,6 @@ type t = { gh_token : string option; offline : string option; status_rules : status_rules; - suppress_cancelled_events : bool; } let make (json_config : Notabot_t.config) (secrets : Notabot_t.secrets) = @@ -74,13 +73,15 @@ let make (json_config : Notabot_t.config) (secrets : Notabot_t.secrets) = (if j.failure then Some (State Failure) else None); (if j.pending then Some (State Pending) else None); (if j.error then Some (State Error) else None); - (if j.cancelled then Some Cancelled else None); + ( match j.cancelled with + | Some r -> Some (Cancelled r) + | None -> None + ); (if j.consecutive_success then Some ConsecutiveSuccess else None); ] in { title = json_config.status_rules.title; status } in - let suppress_cancelled_events = Option.default true json_config.suppress_cancelled_events in { chans; prefix_rules = json_config.prefix_rules; @@ -90,7 +91,6 @@ let make (json_config : Notabot_t.config) (secrets : Notabot_t.secrets) = gh_token = secrets.gh_token; offline = json_config.offline; status_rules; - suppress_cancelled_events; } let load path secrets = diff --git a/lib/notabot.atd b/lib/notabot.atd index 334cc9a9..fb05cc9c 100644 --- a/lib/notabot.atd +++ b/lib/notabot.atd @@ -56,7 +56,7 @@ type status_state = { failure: bool; pending: bool; error: bool; - cancelled: bool; + ?cancelled: string option; consecutive_success: bool; } @@ -69,7 +69,6 @@ type config = { prefix_rules : prefix_config; label_rules : label_config; status_rules : status_config; - ?suppress_cancelled_events : bool option; (* supresses status cancelled events; if not specified - true *) ?main_branch_name: string option; (* used to filter out notifications about merges of main branch into other branches *) ?offline: string option; (* where to find github api data when http calls are not allowed *) } diff --git a/test/notabot.json b/test/notabot.json index fc41fe06..07b8fc90 100644 --- a/test/notabot.json +++ b/test/notabot.json @@ -11,7 +11,7 @@ "success": true, "failure": true, "error": true, - "cancelled": false, + "cancelled": "^\\(Build #[0-9]+ canceled by .+\\|Failed (exit status 255)\\)$", "consecutive_success": false } }, From ba3d6914b734b69507c8229185555ff401bb5365 Mon Sep 17 00:00:00 2001 From: Haliq Date: Mon, 27 Jul 2020 16:11:15 +0800 Subject: [PATCH 03/39] made success variant status rule --- lib/action.ml | 14 ++++++++++---- lib/config.ml | 9 ++++++--- lib/notabot.atd | 9 +++++++-- test/notabot.json | 3 +-- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index 6dd03d0e..23849566 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -176,9 +176,9 @@ let hide_cancelled (notification : status_notification) cfg = ) let hide_success (n : status_notification) (ctx : Context.t) = - match List.exists ctx.cfg.status_rules.status ~f:(Poly.equal ConsecutiveSuccess) with - | true -> false - | false -> + match List.exists ctx.cfg.status_rules.status ~f:(Poly.equal HideConsecutiveSuccess) with + | false -> false + | true -> match n.state with | Success -> List.exists @@ -206,7 +206,13 @@ let partition_status (ctx : Context.t) (n : status_notification) = | false -> Lwt.return (partition_commit cfg commit.files) in let res = - match List.exists cfg.status_rules.status ~f:(Poly.equal (State n.state)) with + match + List.exists cfg.status_rules.status ~f:(fun x -> + match x with + | State _ as s -> Poly.equal s (State n.state) + | HideConsecutiveSuccess -> Poly.equal Success n.state + | _ -> false) + with | false -> Lwt.return [] | true -> match List.exists ~f:id [ hide_cancelled n cfg; hide_success n ctx ] with diff --git a/lib/config.ml b/lib/config.ml index 245ffbb1..2d5584f1 100644 --- a/lib/config.ml +++ b/lib/config.ml @@ -4,7 +4,7 @@ module Chan_map = Map.Make (String) type config_status_state = | State of Github_t.status_state | Cancelled of string - | ConsecutiveSuccess + | HideConsecutiveSuccess type status_rules = { title : string list option; @@ -69,7 +69,11 @@ let make (json_config : Notabot_t.config) (secrets : Notabot_t.secrets) = let j = json_config.status_rules.status in List.filter_map id [ - (if j.success then Some (State Success) else None); + ( match j.success with + | ShowAll -> Some (State Success) + | HideAll -> None + | HideConsecutive -> Some HideConsecutiveSuccess + ); (if j.failure then Some (State Failure) else None); (if j.pending then Some (State Pending) else None); (if j.error then Some (State Error) else None); @@ -77,7 +81,6 @@ let make (json_config : Notabot_t.config) (secrets : Notabot_t.secrets) = | Some r -> Some (Cancelled r) | None -> None ); - (if j.consecutive_success then Some ConsecutiveSuccess else None); ] in { title = json_config.status_rules.title; status } diff --git a/lib/notabot.atd b/lib/notabot.atd index fb05cc9c..ace1d6c7 100644 --- a/lib/notabot.atd +++ b/lib/notabot.atd @@ -51,13 +51,18 @@ type label_config = { rules: label_rule list; } +type success_status_rule = [ + | ShowAll + | HideAll + | HideConsecutive +] + type status_state = { - success: bool; + success: success_status_rule; failure: bool; pending: bool; error: bool; ?cancelled: string option; - consecutive_success: bool; } type status_config = { diff --git a/test/notabot.json b/test/notabot.json index 07b8fc90..b1aa1fa9 100644 --- a/test/notabot.json +++ b/test/notabot.json @@ -8,11 +8,10 @@ ], "status": { "pending": false, - "success": true, "failure": true, "error": true, "cancelled": "^\\(Build #[0-9]+ canceled by .+\\|Failed (exit status 255)\\)$", - "consecutive_success": false + "success": "HideConsecutive" } }, "prefix_rules": { From 04a395d3b8bebbc3f001e548a70d15c57b781ace Mon Sep 17 00:00:00 2001 From: Haliq Date: Tue, 28 Jul 2020 15:03:26 +0800 Subject: [PATCH 04/39] used github state directly rather than config state rule variant --- lib/action.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/action.ml b/lib/action.ml index 23849566..8d0d7554 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -209,7 +209,7 @@ let partition_status (ctx : Context.t) (n : status_notification) = match List.exists cfg.status_rules.status ~f:(fun x -> match x with - | State _ as s -> Poly.equal s (State n.state) + | State s -> Poly.equal s n.state | HideConsecutiveSuccess -> Poly.equal Success n.state | _ -> false) with From 14d6d8478f92038d544adfdfba2e4bd6913413d1 Mon Sep 17 00:00:00 2001 From: Haliq Date: Wed, 29 Jul 2020 17:00:20 +0800 Subject: [PATCH 05/39] implement variant for prefix match results tracking prefix length, use longest matching win heuristic, implement use in partition functions --- lib/action.ml | 91 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 56 insertions(+), 35 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index bd85df92..c2f6a37c 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -9,9 +9,51 @@ open Github_j let log = Log.from "action" -let touching_prefix rule name = - let has_prefix s = List.exists ~f:(fun prefix -> String.is_prefix s ~prefix) in - (List.is_empty rule.prefix || has_prefix name rule.prefix) && not (has_prefix name rule.ignore) +type prefix_match = + | Match of int + | NoMatch + +let chan_of_prefix_rule (r : prefix_rule) = Some r.chan + +let touching_prefix (rule : Notabot_t.prefix_rule) name = + let match_lengths s ps = + List.filter_map ~f:(fun prefix -> if String.is_prefix s ~prefix then Some (String.length prefix) else None) ps + in + let has_prefix s ps = not @@ List.is_empty @@ match_lengths s ps in + match has_prefix name rule.ignore with + | false -> NoMatch + | true -> + match List.is_empty rule.prefix with + | true -> Match 0 + | false -> + match List.max_elt (match_lengths name rule.prefix) ~compare:( - ) with + | Some x -> Match x + | None -> NoMatch + +let longest_touching_prefix_rule rules name = + let get_m rule = touching_prefix rule name in + let reduce_to_longest_match acc r' = + let _, m = acc in + let m' = get_m r' in + let acc' = r', m' in + match m with + | NoMatch -> acc' + | Match x -> + match m' with + | NoMatch -> acc + | Match y -> if y > x then acc' else acc + in + match rules with + | [] -> None + | x :: xs -> + match List.fold_left xs ~init:(x, get_m x) ~f:reduce_to_longest_match with + | _, NoMatch -> None + | r, Match _ -> Some r + +let chan_of_file rules file = Option.bind ~f:chan_of_prefix_rule @@ longest_touching_prefix_rule rules file + +let unique_chans_of_files rules files = + List.dedup_and_sort ~compare:String.compare @@ List.filter_map files ~f:(chan_of_file rules) let touching_label rule name = let name_lc = String.lowercase name in @@ -31,14 +73,8 @@ let is_main_merge_message ~msg:message ~branch cfg = | _ -> false let filter_push rules commit = - let matching_push rule files = List.exists files ~f:(fun file -> touching_prefix rule file) in - List.filter_map rules ~f:(fun rule -> - let filter = - matching_push rule commit.added || matching_push rule commit.removed || matching_push rule commit.modified - in - match filter with - | false -> None - | true -> Some (rule.chan, commit)) + let files = List.concat [ commit.added; commit.removed; commit.modified ] in + List.map ~f:(fun chan -> chan, commit) @@ unique_chans_of_files rules files let group_commit webhook l = List.filter_map l ~f:(fun (chan, commit) -> @@ -131,29 +167,14 @@ let partition_pr_review cfg (n : pr_review_notification) = | Submitted, _, _ -> partition_label cfg n.pull_request.labels | _ -> [] -let filter_commit rules filename = - rules - |> List.filter_map ~f:(fun rule -> - match touching_prefix rule filename with - | false -> None - | true -> Some rule.chan) - let partition_commit cfg files = - let default = Option.value_map cfg.prefix_rules.default ~default:[] ~f:(fun webhook -> [ webhook ]) in - match files with + let names = List.map ~f:(fun f -> f.filename) files in + match unique_chans_of_files cfg.prefix_rules.rules names with + | _ :: _ as xs -> xs | [] -> - log#error "this commit contains no files"; - [] - | files -> - let rules = cfg.prefix_rules.rules in - let channels = - files - |> List.map ~f:(fun file -> - match filter_commit rules file.filename with - | [] -> default - | l -> l) - in - List.dedup_and_sort ~compare:String.compare (List.concat channels) + match cfg.prefix_rules.default with + | Some webhook -> [ webhook ] + | None -> [] let hide_cancelled (notification : status_notification) cfg = let is_cancelled_status = @@ -218,9 +239,9 @@ let partition_commit_comment cfg n = | Some commit -> Lwt.return (partition_commit cfg commit.files) ) | Some p -> - match filter_commit cfg.prefix_rules.rules p with - | [] -> Lwt.return default - | l -> Lwt.return l + match chan_of_file cfg.prefix_rules.rules p with + | None -> Lwt.return default + | Some chan -> Lwt.return [ chan ] let generate_notifications (ctx : Context.t) req = let cfg = ctx.cfg in From aa764b8c8b868b291821c0007f7821f082cb905f Mon Sep 17 00:00:00 2001 From: Haliq Date: Wed, 29 Jul 2020 17:06:17 +0800 Subject: [PATCH 06/39] use functor instead of monad --- lib/action.ml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index c2f6a37c..25dcc90d 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -13,7 +13,7 @@ type prefix_match = | Match of int | NoMatch -let chan_of_prefix_rule (r : prefix_rule) = Some r.chan +let chan_of_prefix_rule (r : prefix_rule) = r.chan let touching_prefix (rule : Notabot_t.prefix_rule) name = let match_lengths s ps = @@ -50,7 +50,7 @@ let longest_touching_prefix_rule rules name = | _, NoMatch -> None | r, Match _ -> Some r -let chan_of_file rules file = Option.bind ~f:chan_of_prefix_rule @@ longest_touching_prefix_rule rules file +let chan_of_file rules file = Option.(longest_touching_prefix_rule rules file >>| chan_of_prefix_rule) let unique_chans_of_files rules files = List.dedup_and_sort ~compare:String.compare @@ List.filter_map files ~f:(chan_of_file rules) From f6d599b3705ce9d67af48ae7dc7b94a65c3b3206 Mon Sep 17 00:00:00 2001 From: Haliq Date: Mon, 3 Aug 2020 09:52:09 +0800 Subject: [PATCH 07/39] fix wrong pattern matching for prefix ignore --- lib/action.ml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index 25dcc90d..7362ac71 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -21,8 +21,8 @@ let touching_prefix (rule : Notabot_t.prefix_rule) name = in let has_prefix s ps = not @@ List.is_empty @@ match_lengths s ps in match has_prefix name rule.ignore with - | false -> NoMatch - | true -> + | true -> NoMatch + | false -> match List.is_empty rule.prefix with | true -> Match 0 | false -> @@ -45,7 +45,7 @@ let longest_touching_prefix_rule rules name = in match rules with | [] -> None - | x :: xs -> + | (x : prefix_rule) :: xs -> match List.fold_left xs ~init:(x, get_m x) ~f:reduce_to_longest_match with | _, NoMatch -> None | r, Match _ -> Some r From 1d5e25335b205c5789415b4e16b09330b7fbd4e9 Mon Sep 17 00:00:00 2001 From: Haliq Date: Mon, 3 Aug 2020 15:08:05 +0800 Subject: [PATCH 08/39] rename arguments --- lib/action.ml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index 7362ac71..2c2b3120 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -16,10 +16,12 @@ type prefix_match = let chan_of_prefix_rule (r : prefix_rule) = r.chan let touching_prefix (rule : Notabot_t.prefix_rule) name = - let match_lengths s ps = - List.filter_map ~f:(fun prefix -> if String.is_prefix s ~prefix then Some (String.length prefix) else None) ps + let match_lengths filename prefixes = + List.filter_map + ~f:(fun prefix -> if String.is_prefix filename ~prefix then Some (String.length prefix) else None) + prefixes in - let has_prefix s ps = not @@ List.is_empty @@ match_lengths s ps in + let has_prefix filename prefixes = not @@ List.is_empty @@ match_lengths filename prefixes in match has_prefix name rule.ignore with | true -> NoMatch | false -> From e86b4a4cba29cc22aa5c9373d4958dc4eac60a3d Mon Sep 17 00:00:00 2001 From: Haliq Date: Mon, 3 Aug 2020 15:09:02 +0800 Subject: [PATCH 09/39] use list pattern matching --- lib/action.ml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index 2c2b3120..6214e6cd 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -25,9 +25,9 @@ let touching_prefix (rule : Notabot_t.prefix_rule) name = match has_prefix name rule.ignore with | true -> NoMatch | false -> - match List.is_empty rule.prefix with - | true -> Match 0 - | false -> + match rule.prefix with + | [] -> Match 0 + | _ -> match List.max_elt (match_lengths name rule.prefix) ~compare:( - ) with | Some x -> Match x | None -> NoMatch From aac77ca597d457f4ec24dec13bd4406e6004e050 Mon Sep 17 00:00:00 2001 From: Haliq Date: Mon, 3 Aug 2020 15:10:14 +0800 Subject: [PATCH 10/39] remove redundant has_prefix --- lib/action.ml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index 6214e6cd..e802c28f 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -21,10 +21,9 @@ let touching_prefix (rule : Notabot_t.prefix_rule) name = ~f:(fun prefix -> if String.is_prefix filename ~prefix then Some (String.length prefix) else None) prefixes in - let has_prefix filename prefixes = not @@ List.is_empty @@ match_lengths filename prefixes in - match has_prefix name rule.ignore with - | true -> NoMatch - | false -> + match match_lengths name rule.ignore with + | _ :: _ -> NoMatch + | [] -> match rule.prefix with | [] -> Match 0 | _ -> From b17f642663320b2fa478df65eabbf9afeb7af78f Mon Sep 17 00:00:00 2001 From: Haliq Date: Mon, 3 Aug 2020 15:11:06 +0800 Subject: [PATCH 11/39] use Int.compare instead of minus --- lib/action.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/action.ml b/lib/action.ml index e802c28f..cc06ab63 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -27,7 +27,7 @@ let touching_prefix (rule : Notabot_t.prefix_rule) name = match rule.prefix with | [] -> Match 0 | _ -> - match List.max_elt (match_lengths name rule.prefix) ~compare:( - ) with + match List.max_elt (match_lengths name rule.prefix) ~compare:Int.compare with | Some x -> Match x | None -> NoMatch From 1bcbee96f7e9bb2dcf715aca0de8c0b27bcb8cf0 Mon Sep 17 00:00:00 2001 From: Haliq Date: Mon, 3 Aug 2020 15:12:33 +0800 Subject: [PATCH 12/39] replace fmap with Option.map --- lib/action.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/action.ml b/lib/action.ml index cc06ab63..18e4aa9f 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -51,7 +51,7 @@ let longest_touching_prefix_rule rules name = | _, NoMatch -> None | r, Match _ -> Some r -let chan_of_file rules file = Option.(longest_touching_prefix_rule rules file >>| chan_of_prefix_rule) +let chan_of_file rules file = Option.map ~f:chan_of_prefix_rule @@ longest_touching_prefix_rule rules file let unique_chans_of_files rules files = List.dedup_and_sort ~compare:String.compare @@ List.filter_map files ~f:(chan_of_file rules) From 7bf771f344e9df84e0843691003c6de149163c6c Mon Sep 17 00:00:00 2001 From: Haliq Date: Mon, 3 Aug 2020 15:16:07 +0800 Subject: [PATCH 13/39] rename arguments and variables to contextualize --- lib/action.ml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index 18e4aa9f..318bd559 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -33,16 +33,16 @@ let touching_prefix (rule : Notabot_t.prefix_rule) name = let longest_touching_prefix_rule rules name = let get_m rule = touching_prefix rule name in - let reduce_to_longest_match acc r' = - let _, m = acc in - let m' = get_m r' in - let acc' = r', m' in - match m with - | NoMatch -> acc' + let reduce_to_longest_match longest_rule_match_pair current_rule = + let _, longest_match = longest_rule_match_pair in + let current_match = get_m current_rule in + let current_rule_match_pair = current_rule, current_match in + match longest_match with + | NoMatch -> current_rule_match_pair | Match x -> - match m' with - | NoMatch -> acc - | Match y -> if y > x then acc' else acc + match current_match with + | NoMatch -> longest_rule_match_pair + | Match y -> if y > x then current_rule_match_pair else longest_rule_match_pair in match rules with | [] -> None From 1c2e97abe875d9bf6810e58864e79ae4c27539d5 Mon Sep 17 00:00:00 2001 From: Haliq Date: Mon, 3 Aug 2020 15:30:17 +0800 Subject: [PATCH 14/39] rename symbols to contextualize, specify constructor module Config, use find_map instead of fold --- lib/action.ml | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index 8d0d7554..2f72f526 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -156,15 +156,12 @@ let partition_commit cfg files = List.dedup_and_sort ~compare:String.compare (List.concat channels) let hide_cancelled (notification : status_notification) cfg = - let f a x = - match a with - | Some _ as v -> v - | None -> - match x with - | Cancelled r -> Some r + let find_cancelled status_state = + match status_state with + | Config.Cancelled r -> Some r | _ -> None in - let regexp_opt = List.fold cfg.status_rules.status ~init:None ~f in + let regexp_opt = List.find_map cfg.status_rules.status ~f:find_cancelled in match regexp_opt with | None -> false | Some regexp -> @@ -176,7 +173,7 @@ let hide_cancelled (notification : status_notification) cfg = ) let hide_success (n : status_notification) (ctx : Context.t) = - match List.exists ctx.cfg.status_rules.status ~f:(Poly.equal HideConsecutiveSuccess) with + match List.exists ctx.cfg.status_rules.status ~f:(Poly.equal Config.HideConsecutiveSuccess) with | false -> false | true -> match n.state with From 0c19d608b9eadfbb6c75e1bea6da81b0df53dd86 Mon Sep 17 00:00:00 2001 From: Haliq Date: Mon, 3 Aug 2020 15:34:33 +0800 Subject: [PATCH 15/39] add documentation for cancelled status_state --- lib/notabot.atd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/notabot.atd b/lib/notabot.atd index ace1d6c7..22b65a5c 100644 --- a/lib/notabot.atd +++ b/lib/notabot.atd @@ -62,7 +62,7 @@ type status_state = { failure: bool; pending: bool; error: bool; - ?cancelled: string option; + ?cancelled: string option; (* if specified, it will use the string as regex to match the description from the payload to determine if the status_state is cancelled *) } type status_config = { From 9afff9aa1b0a3e649687a2b71e7e0286ff3a84ae Mon Sep 17 00:00:00 2001 From: Haliq Date: Tue, 4 Aug 2020 10:13:15 +0800 Subject: [PATCH 16/39] add new test for longest match --- .../push.two_commits_longest_match.json | 204 ++++++++++++++++++ test/notabot.json | 9 +- test/secrets.json | 6 +- test/slack_payloads.expected | 37 ++++ 4 files changed, 254 insertions(+), 2 deletions(-) create mode 100644 mock_payloads/push.two_commits_longest_match.json diff --git a/mock_payloads/push.two_commits_longest_match.json b/mock_payloads/push.two_commits_longest_match.json new file mode 100644 index 00000000..3d91cb4b --- /dev/null +++ b/mock_payloads/push.two_commits_longest_match.json @@ -0,0 +1,204 @@ +{ + "ref": "refs/heads/master", + "before": "fb245e2a6d52d10025c8bd4f36f6e3134d85ae18", + "after": "e2173f38ae43865433a182c1fc1b5442d9763b54", + "created": false, + "deleted": false, + "forced": false, + "base_ref": null, + "compare": "https://github.com/thatportugueseguy/monorepo/compare/fb245e2a6d52...e2173f38ae43", + "commits": [ + { + "id": "80452f696a8988e7234063d47720a62e902f2afc", + "tree_id": "bf7530bb0b13d0b9f471ff5d1951c59488b5704c", + "distinct": true, + "message": "Update readme", + "timestamp": "2019-07-06T11:47:56+01:00", + "url": "https://github.com/thatportugueseguy/monorepo/commit/80452f696a8988e7234063d47720a62e902f2afc", + "author": { + "name": "jose nogueira", + "email": "mail@example.org", + "username": "thatportugueseguy" + }, + "committer": { + "name": "jose nogueira", + "email": "mail@example.org", + "username": "thatportugueseguy" + }, + "added": [], + "removed": [], + "modified": [ + "README.md" + ] + }, + { + "id": "e2173f38ae43865433a182c1fc1b5442d9763b54", + "tree_id": "6dc065fdfacfc72cd7b1f5e7a9391c238e4fb74e", + "distinct": true, + "message": "Add TESTING.md", + "timestamp": "2019-07-06T11:48:38+01:00", + "url": "https://github.com/thatportugueseguy/monorepo/commit/e2173f38ae43865433a182c1fc1b5442d9763b54", + "author": { + "name": "jose nogueira", + "email": "mail@example.org", + "username": "thatportugueseguy" + }, + "committer": { + "name": "jose nogueira", + "email": "mail@example.org", + "username": "thatportugueseguy" + }, + "added": [ + "TESTING.md", + "backend/a1/longest/test.ml" + ], + "removed": [], + "modified": [] + } + ], + "head_commit": { + "id": "e2173f38ae43865433a182c1fc1b5442d9763b54", + "tree_id": "6dc065fdfacfc72cd7b1f5e7a9391c238e4fb74e", + "distinct": true, + "message": "Add TESTING.md", + "timestamp": "2019-07-06T11:48:38+01:00", + "url": "https://github.com/thatportugueseguy/monorepo/commit/e2173f38ae43865433a182c1fc1b5442d9763b54", + "author": { + "name": "jose nogueira", + "email": "mail@example.org", + "username": "thatportugueseguy" + }, + "committer": { + "name": "jose nogueira", + "email": "mail@example.org", + "username": "thatportugueseguy" + }, + "added": [ + "TESTING.md" + ], + "removed": [], + "modified": [] + }, + "repository": { + "id": 0, + "node_id": "00000000000000000000", + "name": "monorepo", + "full_name": "thatportugueseguy/monorepo", + "private": true, + "owner": { + "name": "thatportugueseguy", + "email": "mail@example.org", + "login": "thatportugueseguy", + "id": 0, + "node_id": "00000000000000000000", + "avatar_url": "https://avatars1.githubusercontent.com/u/000000?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/thatportugueseguy", + "html_url": "https://github.com/thatportugueseguy", + "followers_url": "https://api.github.com/users/thatportugueseguy/followers", + "following_url": "https://api.github.com/users/thatportugueseguy/following{/other_user}", + "gists_url": "https://api.github.com/users/thatportugueseguy/gists{/gist_id}", + "starred_url": "https://api.github.com/users/thatportugueseguy/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/thatportugueseguy/subscriptions", + "organizations_url": "https://api.github.com/users/thatportugueseguy/orgs", + "repos_url": "https://api.github.com/users/thatportugueseguy/repos", + "events_url": "https://api.github.com/users/thatportugueseguy/events{/privacy}", + "received_events_url": "https://api.github.com/users/thatportugueseguy/received_events", + "type": "User", + "site_admin": false + }, + "html_url": "https://github.com/thatportugueseguy/monorepo", + "description": null, + "fork": false, + "url": "https://github.com/thatportugueseguy/monorepo", + "forks_url": "https://api.github.com/repos/thatportugueseguy/monorepo/forks", + "keys_url": "https://api.github.com/repos/thatportugueseguy/monorepo/keys{/key_id}", + "collaborators_url": "https://api.github.com/repos/thatportugueseguy/monorepo/collaborators{/collaborator}", + "teams_url": "https://api.github.com/repos/thatportugueseguy/monorepo/teams", + "hooks_url": "https://api.github.com/repos/thatportugueseguy/monorepo/hooks", + "issue_events_url": "https://api.github.com/repos/thatportugueseguy/monorepo/issues/events{/number}", + "events_url": "https://api.github.com/repos/thatportugueseguy/monorepo/events", + "assignees_url": "https://api.github.com/repos/thatportugueseguy/monorepo/assignees{/user}", + "branches_url": "https://api.github.com/repos/thatportugueseguy/monorepo/branches{/branch}", + "tags_url": "https://api.github.com/repos/thatportugueseguy/monorepo/tags", + "blobs_url": "https://api.github.com/repos/thatportugueseguy/monorepo/git/blobs{/sha}", + "git_tags_url": "https://api.github.com/repos/thatportugueseguy/monorepo/git/tags{/sha}", + "git_refs_url": "https://api.github.com/repos/thatportugueseguy/monorepo/git/refs{/sha}", + "trees_url": "https://api.github.com/repos/thatportugueseguy/monorepo/git/trees{/sha}", + "statuses_url": "https://api.github.com/repos/thatportugueseguy/monorepo/statuses/{sha}", + "languages_url": "https://api.github.com/repos/thatportugueseguy/monorepo/languages", + "stargazers_url": "https://api.github.com/repos/thatportugueseguy/monorepo/stargazers", + "contributors_url": "https://api.github.com/repos/thatportugueseguy/monorepo/contributors", + "subscribers_url": "https://api.github.com/repos/thatportugueseguy/monorepo/subscribers", + "subscription_url": "https://api.github.com/repos/thatportugueseguy/monorepo/subscription", + "commits_url": "https://api.github.com/repos/thatportugueseguy/monorepo/commits{/sha}", + "git_commits_url": "https://api.github.com/repos/thatportugueseguy/monorepo/git/commits{/sha}", + "comments_url": "https://api.github.com/repos/thatportugueseguy/monorepo/comments{/number}", + "issue_comment_url": "https://api.github.com/repos/thatportugueseguy/monorepo/issues/comments{/number}", + "contents_url": "https://api.github.com/repos/thatportugueseguy/monorepo/contents/{+path}", + "compare_url": "https://api.github.com/repos/thatportugueseguy/monorepo/compare/{base}...{head}", + "merges_url": "https://api.github.com/repos/thatportugueseguy/monorepo/merges", + "archive_url": "https://api.github.com/repos/thatportugueseguy/monorepo/{archive_format}{/ref}", + "downloads_url": "https://api.github.com/repos/thatportugueseguy/monorepo/downloads", + "issues_url": "https://api.github.com/repos/thatportugueseguy/monorepo/issues{/number}", + "pulls_url": "https://api.github.com/repos/thatportugueseguy/monorepo/pulls{/number}", + "milestones_url": "https://api.github.com/repos/thatportugueseguy/monorepo/milestones{/number}", + "notifications_url": "https://api.github.com/repos/thatportugueseguy/monorepo/notifications{?since,all,participating}", + "labels_url": "https://api.github.com/repos/thatportugueseguy/monorepo/labels{/name}", + "releases_url": "https://api.github.com/repos/thatportugueseguy/monorepo/releases{/id}", + "deployments_url": "https://api.github.com/repos/thatportugueseguy/monorepo/deployments", + "created_at": 1562409695, + "updated_at": "2019-07-06T10:42:28Z", + "pushed_at": 1562410128, + "git_url": "git://github.com/thatportugueseguy/monorepo.git", + "ssh_url": "git@github.com:thatportugueseguy/monorepo.git", + "clone_url": "https://github.com/thatportugueseguy/monorepo.git", + "svn_url": "https://github.com/thatportugueseguy/monorepo", + "homepage": null, + "size": 0, + "stargazers_count": 0, + "watchers_count": 0, + "language": null, + "has_issues": true, + "has_projects": true, + "has_downloads": true, + "has_wiki": true, + "has_pages": false, + "forks_count": 0, + "mirror_url": null, + "archived": false, + "disabled": false, + "open_issues_count": 0, + "license": null, + "forks": 0, + "open_issues": 0, + "watchers": 0, + "default_branch": "master", + "stargazers": 0, + "master_branch": "master" + }, + "pusher": { + "name": "thatportugueseguy", + "email": "mail@example.org" + }, + "sender": { + "login": "thatportugueseguy", + "id": 0, + "node_id": "00000000000000000000", + "avatar_url": "https://avatars1.githubusercontent.com/u/000000?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/thatportugueseguy", + "html_url": "https://github.com/thatportugueseguy", + "followers_url": "https://api.github.com/users/thatportugueseguy/followers", + "following_url": "https://api.github.com/users/thatportugueseguy/following{/other_user}", + "gists_url": "https://api.github.com/users/thatportugueseguy/gists{/gist_id}", + "starred_url": "https://api.github.com/users/thatportugueseguy/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/thatportugueseguy/subscriptions", + "organizations_url": "https://api.github.com/users/thatportugueseguy/orgs", + "repos_url": "https://api.github.com/users/thatportugueseguy/repos", + "events_url": "https://api.github.com/users/thatportugueseguy/events{/privacy}", + "received_events_url": "https://api.github.com/users/thatportugueseguy/received_events", + "type": "User", + "site_admin": false + } +} \ No newline at end of file diff --git a/test/notabot.json b/test/notabot.json index f11e940e..81861b7b 100644 --- a/test/notabot.json +++ b/test/notabot.json @@ -23,6 +23,13 @@ "ignore": [], "chan": "a1" }, + { + "prefix": [ + "backend/a1/longest" + ], + "ignore": [], + "chan": "longest-a1" + }, { "prefix": [ "backend/a5", @@ -74,4 +81,4 @@ ] }, "suppress_cancelled_events": true -} +} \ No newline at end of file diff --git a/test/secrets.json b/test/secrets.json index c9a15d34..f29896fd 100644 --- a/test/secrets.json +++ b/test/secrets.json @@ -8,6 +8,10 @@ "url": "https://slack_webhook_url", "channel": "a1" }, + { + "url": "https://slack_webhook_url", + "channel": "longest-a1" + }, { "url": "https://slack_webhook_url", "channel": "backend" @@ -29,4 +33,4 @@ "channel": "a3" } ] -} +} \ No newline at end of file diff --git a/test/slack_payloads.expected b/test/slack_payloads.expected index 8710a0ff..e2e6d6d6 100644 --- a/test/slack_payloads.expected +++ b/test/slack_payloads.expected @@ -429,6 +429,43 @@ will notify #all-push-events } ] } +===== file ../mock_payloads/push.two_commits_longest_match.json ===== +will notify #all-push-events +{ + "text": + " pushed by thatportugueseguy", + "attachments": [ + { + "fallback": "Commit pushed notification", + "mrkdwn_in": [ "fields" ], + "color": "#ccc", + "fields": [ + { + "value": + "`` Update readme - jose nogueira\n`` Add TESTING.md - jose nogueira" + } + ] + } + ] +} +will notify #longest-a1 +{ + "text": + " pushed by thatportugueseguy", + "attachments": [ + { + "fallback": "Commit pushed notification", + "mrkdwn_in": [ "fields" ], + "color": "#ccc", + "fields": [ + { + "value": + "`` Add TESTING.md - jose nogueira" + } + ] + } + ] +} ===== file ../mock_payloads/status.cancelled_test.json ===== ===== file ../mock_payloads/status.failure_test.json ===== will notify #all-push-events From ef871a4eaf1fd3cf356b0d15985ed4567fbbfac2 Mon Sep 17 00:00:00 2001 From: Haliq Date: Wed, 5 Aug 2020 09:45:02 +0800 Subject: [PATCH 17/39] refactor success state into two fields in atd --- lib/config.ml | 8 +++++--- lib/notabot.atd | 9 ++------- test/notabot.json | 3 ++- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/lib/config.ml b/lib/config.ml index 2d5584f1..df630f95 100644 --- a/lib/config.ml +++ b/lib/config.ml @@ -70,9 +70,11 @@ let make (json_config : Notabot_t.config) (secrets : Notabot_t.secrets) = List.filter_map id [ ( match j.success with - | ShowAll -> Some (State Success) - | HideAll -> None - | HideConsecutive -> Some HideConsecutiveSuccess + | false -> None + | true -> + match j.success_once with + | Some true -> Some HideConsecutiveSuccess + | _ -> Some (State Success) ); (if j.failure then Some (State Failure) else None); (if j.pending then Some (State Pending) else None); diff --git a/lib/notabot.atd b/lib/notabot.atd index 22b65a5c..8ed11b32 100644 --- a/lib/notabot.atd +++ b/lib/notabot.atd @@ -51,14 +51,9 @@ type label_config = { rules: label_rule list; } -type success_status_rule = [ - | ShowAll - | HideAll - | HideConsecutive -] - type status_state = { - success: success_status_rule; + success: bool; + ?success_once: bool option; failure: bool; pending: bool; error: bool; diff --git a/test/notabot.json b/test/notabot.json index b1aa1fa9..fa02061c 100644 --- a/test/notabot.json +++ b/test/notabot.json @@ -11,7 +11,8 @@ "failure": true, "error": true, "cancelled": "^\\(Build #[0-9]+ canceled by .+\\|Failed (exit status 255)\\)$", - "success": "HideConsecutive" + "success": true, + "success_once": true } }, "prefix_rules": { From fbe24ac5862957c69afdf9f78c24c1eb0e548d0a Mon Sep 17 00:00:00 2001 From: Haliq Date: Wed, 5 Aug 2020 12:27:27 +0800 Subject: [PATCH 18/39] remove support for heading and recurse transform for bold and emph --- lib/mrkdwn.ml | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/mrkdwn.ml b/lib/mrkdwn.ml index 21ccd534..98d0e1ce 100644 --- a/lib/mrkdwn.ml +++ b/lib/mrkdwn.ml @@ -14,12 +14,10 @@ and surround s t = Raw (Printf.sprintf "%s%s%s" s t s) and transform = function - | H1 t -> H1 (transform_list t) - | H2 t -> H2 (transform_list t) - | H3 t | H4 t | H5 t | H6 t -> Paragraph [ Bold (transform_list t) ] + | H1 t | H2 t | H3 t | H4 t | H5 t | H6 t -> Paragraph (transform_list [ Bold t ]) | Paragraph t -> Paragraph (transform_list t) - | Emph t -> surround "_" t - | Bold t -> surround "*" t + | Emph t -> surround "_" (transform_list t) + | Bold t -> surround "*" (transform_list t) | Ul ts -> Ul (transform_flatten ts) | Ol ts -> Ol (transform_flatten ts) | Ulp ts -> Ulp (transform_flatten ts) @@ -27,7 +25,7 @@ and transform = function | Url (href, label, title) -> let label = escape_url_chars @@ to_markdown @@ transform_list label in let title = if String.length title > 0 then Printf.sprintf "%s - " @@ escape_url_chars title else title in - Raw (Printf.sprintf "<%s%s|%s>" title label href) + Raw (Printf.sprintf "<%s|%s%s>" href title label) | Html _ as e -> Raw (Printf.sprintf "`%s`" @@ to_markdown [ e ]) | Html_comment _ -> Br | Html_block _ as e -> Code_block ("", to_markdown [ e ]) From 26d7bb997724eb2ae1af009f10c7ba7e803becbb Mon Sep 17 00:00:00 2001 From: Haliq Date: Wed, 5 Aug 2020 12:43:14 +0800 Subject: [PATCH 19/39] fix test --- test/slack_payloads.expected | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/slack_payloads.expected b/test/slack_payloads.expected index 8710a0ff..d55c83b0 100644 --- a/test/slack_payloads.expected +++ b/test/slack_payloads.expected @@ -89,7 +89,7 @@ will notify #all-push-events "pretext": " xinyuluo commented on `` add new line at EOF", "text": - "*bold*, _italic_\n\n> blockquote\n\n\n- list-element 1\n- list-element 2\n - list-element2.1\n - \n\n\n```\n
html block
\n```\n```\nStdio.printf \"hello ocaml\"\n```", + "*bold*, _italic_\n\n> blockquote\n\n\n- list-element 1\n- list-element 2\n - list-element2.1\n - \n\n\n```\n
html block
\n```\n```\nStdio.printf \"hello ocaml\"\n```", "footer": "New comment by xinyuluo in " } From a96fc801cf37822d598822e692763d1bf5518b57 Mon Sep 17 00:00:00 2001 From: ip Date: Wed, 19 Aug 2020 14:52:14 -0400 Subject: [PATCH 20/39] specify success state as tristate switch --- lib/common.ml | 12 ++++++++++++ lib/config.ml | 8 +++----- lib/notabot.atd | 5 +++-- test/notabot.json | 5 ++--- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/lib/common.ml b/lib/common.ml index db088ff1..b1e8205c 100644 --- a/lib/common.ml +++ b/lib/common.ml @@ -4,3 +4,15 @@ let first_line s = match String.split ~on:'\n' s with | x :: _ -> x | [] -> s + +module Tristate : Atdgen_runtime.Json_adapter.S = struct + let normalize = function + | `Bool true -> `String "true" + | `Bool false -> `String "false" + | x -> x + + let restore = function + | `String "true" -> `Bool true + | `String "false" -> `Bool false + | x -> x +end diff --git a/lib/config.ml b/lib/config.ml index df630f95..a30aff00 100644 --- a/lib/config.ml +++ b/lib/config.ml @@ -70,11 +70,9 @@ let make (json_config : Notabot_t.config) (secrets : Notabot_t.secrets) = List.filter_map id [ ( match j.success with - | false -> None - | true -> - match j.success_once with - | Some true -> Some HideConsecutiveSuccess - | _ -> Some (State Success) + | False -> None + | True -> Some (State Success) + | Once -> Some HideConsecutiveSuccess ); (if j.failure then Some (State Failure) else None); (if j.pending then Some (State Pending) else None); diff --git a/lib/notabot.atd b/lib/notabot.atd index 8ed11b32..d02d9f2e 100644 --- a/lib/notabot.atd +++ b/lib/notabot.atd @@ -51,9 +51,10 @@ type label_config = { rules: label_rule list; } +type tristate = [ True | False | Once ] + type status_state = { - success: bool; - ?success_once: bool option; + success: tristate; failure: bool; pending: bool; error: bool; diff --git a/test/notabot.json b/test/notabot.json index fa02061c..1de1a05c 100644 --- a/test/notabot.json +++ b/test/notabot.json @@ -11,8 +11,7 @@ "failure": true, "error": true, "cancelled": "^\\(Build #[0-9]+ canceled by .+\\|Failed (exit status 255)\\)$", - "success": true, - "success_once": true + "success": "once" } }, "prefix_rules": { @@ -75,4 +74,4 @@ } ] } -} \ No newline at end of file +} From e681ff362d5321a7abe2d2b871f8338b6010384d Mon Sep 17 00:00:00 2001 From: ip Date: Wed, 19 Aug 2020 14:59:39 -0400 Subject: [PATCH 21/39] use standard json --- lib/dune | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/dune b/lib/dune index 5d125ad7..64c32c45 100644 --- a/lib/dune +++ b/lib/dune @@ -15,7 +15,7 @@ (targets github_j.ml github_j.mli) (deps github.atd) (action - (run atdgen -j %{deps}))) + (run atdgen -j -j-std %{deps}))) (rule (targets slack_t.ml slack_t.mli) @@ -27,7 +27,7 @@ (targets slack_j.ml slack_j.mli) (deps slack.atd) (action - (run atdgen -j %{deps}))) + (run atdgen -j -j-std %{deps}))) (rule (targets notabot_t.ml notabot_t.mli) @@ -39,4 +39,4 @@ (targets notabot_j.ml notabot_j.mli) (deps notabot.atd) (action - (run atdgen -j %{deps}))) + (run atdgen -j -j-std %{deps}))) From 404d0a60416a423b982a098c03df0eb4fa004d3b Mon Sep 17 00:00:00 2001 From: ip Date: Wed, 19 Aug 2020 15:33:15 -0400 Subject: [PATCH 22/39] better logs --- lib/slack.ml | 5 +---- src/notabot.ml | 2 +- src/request_handler.ml | 7 ++++--- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/slack.ml b/lib/slack.ml index db60081c..2ba7cf4b 100644 --- a/lib/slack.ml +++ b/lib/slack.ml @@ -369,10 +369,7 @@ let generate_commit_comment_notification cfg notification = Lwt.return notifs let send_notification webhook_url data = - let data = Slack_j.string_of_webhook_notification data in let body = `Raw ("application/json", data) in match%lwt Web.http_request_lwt ~verbose:true ~body `POST webhook_url with | `Ok _ -> Lwt.return_unit - | `Error e -> - log#error "error when posting notification to slack: %s" e; - Exn_lwt.fail "unable to send notification to slack" + | `Error e -> Exn_lwt.fail "failed to send notification to slack : %s" e diff --git a/src/notabot.ml b/src/notabot.ml index 3e70d83a..db20eb01 100644 --- a/src/notabot.ml +++ b/src/notabot.ml @@ -27,7 +27,7 @@ let send_slack_notification webhook file = let data = Stdio.In_channel.read_all file in match Slack_j.webhook_notification_of_string data with | exception exn -> log#error ~exn "unable to parse notification" - | data -> Lwt_main.run (Slack.send_notification webhook data) + | _ -> Lwt_main.run (Slack.send_notification webhook data) let check_common file print config secrets state_path = let ctx = get_context state_path config secrets in diff --git a/src/request_handler.ml b/src/request_handler.ml index b2cdcb80..fd4cf29e 100644 --- a/src/request_handler.ml +++ b/src/request_handler.ml @@ -13,7 +13,9 @@ let process_github_notification (ctx : Lib.Context.t) headers body = Lwt_list.iter_s (fun (chan, msg) -> let url = Config.Chan_map.find chan cfg.chans in - Slack.send_notification url msg) + let data = Slack_j.string_of_webhook_notification msg in + log#info "sending to %s : %s" chan data; + Slack.send_notification url data) notifications let setup_http ~ctx ~signature ~port ~ip = @@ -36,7 +38,6 @@ let setup_http ~ctx ~signature ~port ~ip = body @@ serve ~status ?extra request typ r in let ret_err status s = body @@ serve_text ~status request s in - log#info "http: %s" (show_request request); try%lwt let path = match Stre.nsplitc request.path '/' with @@ -46,7 +47,7 @@ let setup_http ~ctx ~signature ~port ~ip = match request.meth, List.map Web.urldecode path with | _, [ "stats" ] -> ret @@ Lwt.return (sprintf "%s %s uptime\n" signature Devkit.Action.uptime#get_str) | _, [ "github" ] -> - log#info "%s" request.body; + log#info "body: %s" request.body; let%lwt () = process_github_notification ctx request.headers request.body in ret (Lwt.return "ok") | _, _ -> From db2d41c73b17aacbbdfcd58e6a7c3e8dd9945b26 Mon Sep 17 00:00:00 2001 From: ip Date: Wed, 19 Aug 2020 16:10:03 -0400 Subject: [PATCH 23/39] filter out merges of main branch into feature after feature branch is merged into main --- lib/action.ml | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index c6ef3be4..c7cf4611 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -66,6 +66,15 @@ let touching_label rule name = let is_main_merge_message ~msg:message ~branch cfg = match cfg.main_branch_name with + | Some main_branch when String.equal branch main_branch -> + (* + handle "Merge
into " commits when they are merged into main branch + we should have already seen these commits on the feature branch but for some reason they are distinct:true + *) + let prefix = sprintf "Merge branch '%s' into " main_branch in + let prefix2 = sprintf "Merge remote-tracking branch 'origin/%s' into " main_branch in + let title = first_line message in + String.is_prefix title ~prefix || String.is_prefix title ~prefix:prefix2 | Some main_branch -> let expect = sprintf "Merge branch '%s' into %s" main_branch branch in let expect2 = sprintf "Merge remote-tracking branch 'origin/%s' into %s" main_branch branch in @@ -77,9 +86,9 @@ let filter_push rules commit = let files = List.concat [ commit.added; commit.removed; commit.modified ] in List.map ~f:(fun chan -> chan, commit) @@ unique_chans_of_files rules files -let group_commit webhook l = - List.filter_map l ~f:(fun (chan, commit) -> - match String.equal webhook chan with +let group_commit chan l = + List.filter_map l ~f:(fun (chan', commit) -> + match String.equal chan chan' with | false -> None | true -> Some commit) @@ -98,15 +107,15 @@ let partition_push cfg n = match filter_push rules commit with | [] -> default commit | l -> l) + |> List.concat in - let concat_chan = List.concat channels in let prefix_chans = let chans = List.map rules ~f:(fun (rule : prefix_rule) -> rule.chan) in let chans = Option.value_map cfg.prefix_rules.default ~default:chans ~f:(fun default -> default :: chans) in List.dedup_and_sort chans ~compare:String.compare in List.filter_map prefix_chans ~f:(fun chan -> - match group_commit chan concat_chan with + match group_commit chan channels with | [] -> None | l -> Some (chan, { n with commits = l })) From 573deb32859bf7a1ceab20c717643577df2d96ba Mon Sep 17 00:00:00 2001 From: ip Date: Wed, 19 Aug 2020 16:52:00 -0400 Subject: [PATCH 24/39] minor improvement for CI notification message --- lib/slack.ml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/slack.ml b/lib/slack.ml index 2ba7cf4b..7aa0e15c 100644 --- a/lib/slack.ml +++ b/lib/slack.ml @@ -292,11 +292,14 @@ let generate_status_notification (notification : status_notification) = | Some s -> Some (sprintf "*Description*: %s." s) in let commit_info = - sprintf "*Commit*: `<%s|%s>` %s - %s" html_url (git_short_sha_hash sha) (first_line message) author.login + [ sprintf "*Commit*: `<%s|%s>` %s - %s" html_url (git_short_sha_hash sha) (first_line message) author.login ] in let branches_info = - let branches = notification.branches |> List.map ~f:(fun ({ name } : branch) -> name) |> String.concat ~sep:", " in - sprintf "*Branches*: %s" branches + match notification.branches with + | [] -> [] (* happens when branch is force-pushed by the time CI notification arrives *) + | branches -> + let branches = List.map branches ~f:(fun ({ name } : branch) -> name) |> String.concat ~sep:", " in + [ sprintf "*Branches*: %s" branches ] in let summary = match target_url with @@ -320,7 +323,8 @@ let generate_status_notification (notification : status_notification) = pretext = summary; color = Some color_info; text = description_info; - fields = Some [ { title = None; value = String.concat ~sep:"\n" [ commit_info; branches_info ] } ]; + fields = + Some [ { title = None; value = String.concat ~sep:"\n" @@ List.concat [ commit_info; branches_info ] } ]; }; ]; blocks = None; From a632cac400e4512da41a84a2596ffbaf3f1f222b Mon Sep 17 00:00:00 2001 From: ip Date: Wed, 19 Aug 2020 17:01:18 -0400 Subject: [PATCH 25/39] better branch list in CI notification (fix #73) --- lib/action.ml | 2 +- lib/slack.ml | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index c7cf4611..aa7a280c 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -295,7 +295,7 @@ let generate_notifications (ctx : Context.t) req = Lwt.return notifs | Status n -> let%lwt webhooks = partition_status ctx n in - let notifs = List.map ~f:(fun webhook -> webhook, generate_status_notification n) webhooks in + let notifs = List.map ~f:(fun webhook -> webhook, generate_status_notification cfg n) webhooks in Lwt.return notifs | _ -> Lwt.return [] diff --git a/lib/slack.ml b/lib/slack.ml index 7aa0e15c..695ceaa9 100644 --- a/lib/slack.ml +++ b/lib/slack.ml @@ -267,7 +267,7 @@ let generate_push_notification notification = blocks = None; } -let generate_status_notification (notification : status_notification) = +let generate_status_notification (cfg : Config.t) (notification : status_notification) = let { commit; state; description; target_url; context; repository; _ } = notification in let ({ commit : inner_commit; sha; author; html_url; _ } : status_commit) = commit in let ({ message; _ } : inner_commit) = commit in @@ -295,11 +295,14 @@ let generate_status_notification (notification : status_notification) = [ sprintf "*Commit*: `<%s|%s>` %s - %s" html_url (git_short_sha_hash sha) (first_line message) author.login ] in let branches_info = - match notification.branches with + match List.map ~f:(fun { name } -> name) notification.branches with | [] -> [] (* happens when branch is force-pushed by the time CI notification arrives *) | branches -> - let branches = List.map branches ~f:(fun ({ name } : branch) -> name) |> String.concat ~sep:", " in - [ sprintf "*Branches*: %s" branches ] + match cfg.main_branch_name with + | Some main when List.mem branches main ~equal:String.equal -> + (* happens when new branches are branched before CI finishes *) + [ sprintf "*Branch*: %s" main ] + | _ -> [ sprintf "*Branches*: %s" (String.concat ~sep:", " branches) ] in let summary = match target_url with From 2f8895c7754a0826803e36d9b0580e9750070b5e Mon Sep 17 00:00:00 2001 From: ip Date: Fri, 4 Sep 2020 18:24:32 -0400 Subject: [PATCH 26/39] issues: show body only for "new issue" notification --- lib/mrkdwn.ml | 7 ++--- lib/slack.ml | 50 +++++++++++++++--------------------- test/slack_payloads.expected | 11 +++----- 3 files changed, 26 insertions(+), 42 deletions(-) diff --git a/lib/mrkdwn.ml b/lib/mrkdwn.ml index 98d0e1ce..e7ff7d8b 100644 --- a/lib/mrkdwn.ml +++ b/lib/mrkdwn.ml @@ -13,6 +13,7 @@ and surround s t = let t = to_markdown @@ transform_list t in Raw (Printf.sprintf "%s%s%s" s t s) +(** massage markdown AST so that rendered result looks like slack mrkdwn *) and transform = function | H1 t | H2 t | H3 t | H4 t | H5 t | H6 t -> Paragraph (transform_list [ Bold t ]) | Paragraph t -> Paragraph (transform_list t) @@ -34,8 +35,4 @@ and transform = function | Code_block (_, str) -> Code_block ("", str) | (Text _ | Code _ | Br | Hr | NL | Ref _ | Img_ref _ | Raw _ | Raw_block _ | X _) as e -> e -let of_doc t = transform_list t - -let to_mrkdwn doc = to_markdown @@ of_doc doc - -let mrkdwn_of_markdown str = to_mrkdwn @@ of_string str +let mrkdwn_of_markdown str = to_markdown @@ transform_list @@ of_string str diff --git a/lib/slack.ml b/lib/slack.ml index 695ceaa9..079f8953 100644 --- a/lib/slack.ml +++ b/lib/slack.ml @@ -28,24 +28,21 @@ let empty_attachments = let mrkdwn_of_markdown str = String.strip @@ Mrkdwn.mrkdwn_of_markdown str -let mrkdwn_of_markdown_opt str_opt = Option.map ~f:mrkdwn_of_markdown str_opt +let mrkdwn_of_markdown_opt = Option.map ~f:mrkdwn_of_markdown + +let show_labels = function + | [] -> None + | labels -> Some (sprintf "Labels: %s" @@ String.concat ~sep:", " (List.map ~f:(fun x -> x.name) labels)) let generate_pull_request_notification notification = let { action; number; sender; pull_request; repository } = notification in let ({ body; title; html_url; labels; _ } : pull_request) = pull_request in - let label_str = - match labels with - | [] -> None - | labels -> - let value = String.concat ~sep:", " (List.map ~f:(fun x -> x.name) labels) in - Some (sprintf "Labels: %s" value) - in - let action_str = + let action, body = match action with - | Opened -> "opened" - | Closed -> "closed" - | Reopened -> "reopened" - | Labeled -> "labeled" + | Opened -> "opened", Some body + | Closed -> "closed", None + | Reopened -> "reopened", None + | Labeled -> "labeled", show_labels labels | _ -> invalid_arg (sprintf "Notabot doesn't know how to generate notification for the unexpected event %s" @@ -54,7 +51,7 @@ let generate_pull_request_notification notification = let summary = Some (sprintf "<%s|[%s]> Pull request #%d <%s|%s> %s by %s" repository.url repository.full_name number html_url title - action_str sender.login) + action sender.login) in { text = None; @@ -67,12 +64,7 @@ let generate_pull_request_notification notification = fallback = summary; color = Some "#ccc"; pretext = summary; - text = - ( match action with - | Labeled -> label_str - | Closed -> None - | _ -> Some (mrkdwn_of_markdown body) - ); + text = mrkdwn_of_markdown_opt body; }; ]; blocks = None; @@ -158,13 +150,13 @@ let generate_pr_review_comment_notification notification = let generate_issue_notification notification = let ({ action; sender; issue; repository } : issue_notification) = notification in - let { number; body; title; html_url; _ } = issue in - let action_str = + let { number; body; title; html_url; labels; _ } = issue in + let action, body = match action with - | Opened -> "opened" - | Closed -> "closed" - | Reopened -> "reopened" - | Labeled -> "labeled" + | Opened -> "opened", Some body + | Closed -> "closed", None + | Reopened -> "reopened", None + | Labeled -> "labeled", show_labels labels | _ -> invalid_arg (sprintf "Notabot doesn't know how to generate notification for the unexpected event %s" @@ -172,8 +164,8 @@ let generate_issue_notification notification = in let summary = Some - (sprintf "<%s|[%s]> Issue #%d <%s|%s> %s by %s" repository.url repository.full_name number html_url title - action_str sender.login) + (sprintf "<%s|[%s]> Issue #%d <%s|%s> %s by %s" repository.url repository.full_name number html_url title action + sender.login) in { text = None; @@ -186,7 +178,7 @@ let generate_issue_notification notification = fallback = summary; color = Some "#ccc"; pretext = summary; - text = Some (mrkdwn_of_markdown body); + text = mrkdwn_of_markdown_opt body; }; ]; blocks = None; diff --git a/test/slack_payloads.expected b/test/slack_payloads.expected index 7d7cf2c3..f2c64614 100644 --- a/test/slack_payloads.expected +++ b/test/slack_payloads.expected @@ -199,9 +199,7 @@ will notify #backend "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " Issue #6 closed by xinyuluo", - "text": - "consider multiple cases including labeled/opened/closed/reopened" + " Issue #6 closed by xinyuluo" } ] } @@ -216,8 +214,7 @@ will notify #backend "color": "#ccc", "pretext": " Issue #6 labeled by xinyuluo", - "text": - "consider multiple cases including labeled/opened/closed/reopened" + "text": "Labels: backend" } ] } @@ -247,9 +244,7 @@ will notify #backend "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " Issue #6 reopened by xinyuluo", - "text": - "consider multiple cases including labeled/opened/closed/reopened" + " Issue #6 reopened by xinyuluo" } ] } From d29d6a6a8c3b112a3a5878208a8c7bc18803092a Mon Sep 17 00:00:00 2001 From: ip Date: Mon, 14 Sep 2020 13:16:11 -0400 Subject: [PATCH 27/39] slack: properly transform img links --- lib/mrkdwn.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mrkdwn.ml b/lib/mrkdwn.ml index e7ff7d8b..f31bc31e 100644 --- a/lib/mrkdwn.ml +++ b/lib/mrkdwn.ml @@ -31,7 +31,7 @@ and transform = function | Html_comment _ -> Br | Html_block _ as e -> Code_block ("", to_markdown [ e ]) | Blockquote t -> Blockquote (transform_list t) - | Img (alt, src, title) -> Url (src, [ Text alt ], title) + | Img (alt, src, title) -> transform @@ Url (src, [ Text alt ], title) | Code_block (_, str) -> Code_block ("", str) | (Text _ | Code _ | Br | Hr | NL | Ref _ | Img_ref _ | Raw _ | Raw_block _ | X _) as e -> e From 9430abd2d52624812c931f45dd544bdc1392f997 Mon Sep 17 00:00:00 2001 From: ip Date: Tue, 15 Sep 2020 12:17:28 -0400 Subject: [PATCH 28/39] slack: tweak escaping --- lib/mrkdwn.ml | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/mrkdwn.ml b/lib/mrkdwn.ml index f31bc31e..6dcc32a1 100644 --- a/lib/mrkdwn.ml +++ b/lib/mrkdwn.ml @@ -1,9 +1,21 @@ open Omd open Base -let escape_url_chars str = - let repl c = String.substr_replace_all ~pattern:(Char.to_string c) ~with_:(Printf.sprintf "\\%c" c) in - str |> repl '<' |> repl '>' |> repl '|' +let escape_url_chars = Staged.unstage @@ String.Escaping.escape ~escapeworthy:[ '<'; '>'; '|' ] ~escape_char:'\\' + +(* https://api.slack.com/reference/surfaces/formatting#escaping *) +let escape_mrkdwn = + String.concat_map ~f:(function + | '<' -> "<" + | '>' -> ">" + | '&' -> "&" + | c -> String.make 1 c) + +(* omd escapes parentheses in text (bug?) *) +let unescape_omd = + Staged.unstage @@ String.Escaping.unescape_gen_exn ~escapeworthy_map:[ '(', '('; ')', ')' ] ~escape_char:'\\' + +let transform_text s = escape_mrkdwn @@ unescape_omd s let rec transform_list = List.map ~f:transform @@ -33,6 +45,7 @@ and transform = function | Blockquote t -> Blockquote (transform_list t) | Img (alt, src, title) -> transform @@ Url (src, [ Text alt ], title) | Code_block (_, str) -> Code_block ("", str) - | (Text _ | Code _ | Br | Hr | NL | Ref _ | Img_ref _ | Raw _ | Raw_block _ | X _) as e -> e + | Text s -> Text (transform_text s) + | (Code _ | Br | Hr | NL | Ref _ | Img_ref _ | Raw _ | Raw_block _ | X _) as e -> e let mrkdwn_of_markdown str = to_markdown @@ transform_list @@ of_string str From 9d2531f91db22a7df125b396250b3b310f8fca13 Mon Sep 17 00:00:00 2001 From: ip Date: Tue, 15 Sep 2020 12:36:47 -0400 Subject: [PATCH 29/39] slack: fix unescaping of parentheses --- lib/mrkdwn.ml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/mrkdwn.ml b/lib/mrkdwn.ml index 6dcc32a1..6c01b84c 100644 --- a/lib/mrkdwn.ml +++ b/lib/mrkdwn.ml @@ -11,11 +11,10 @@ let escape_mrkdwn = | '&' -> "&" | c -> String.make 1 c) -(* omd escapes parentheses in text (bug?) *) let unescape_omd = Staged.unstage @@ String.Escaping.unescape_gen_exn ~escapeworthy_map:[ '(', '('; ')', ')' ] ~escape_char:'\\' -let transform_text s = escape_mrkdwn @@ unescape_omd s +let transform_text = escape_mrkdwn let rec transform_list = List.map ~f:transform @@ -48,4 +47,5 @@ and transform = function | Text s -> Text (transform_text s) | (Code _ | Br | Hr | NL | Ref _ | Img_ref _ | Raw _ | Raw_block _ | X _) as e -> e -let mrkdwn_of_markdown str = to_markdown @@ transform_list @@ of_string str +(* unescaping here is a workaround of to_markdown escaping parentheses in text (bug?) *) +let mrkdwn_of_markdown str = unescape_omd @@ to_markdown @@ transform_list @@ of_string str From b5851fc40cbae91f2f6fc390d01231104f6beddc Mon Sep 17 00:00:00 2001 From: ip Date: Wed, 30 Sep 2020 13:27:46 -0400 Subject: [PATCH 30/39] highlight author name --- lib/slack.ml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/slack.ml b/lib/slack.ml index 079f8953..8423bc65 100644 --- a/lib/slack.ml +++ b/lib/slack.ml @@ -50,7 +50,7 @@ let generate_pull_request_notification notification = in let summary = Some - (sprintf "<%s|[%s]> Pull request #%d <%s|%s> %s by %s" repository.url repository.full_name number html_url title + (sprintf "<%s|[%s]> Pull request #%d <%s|%s> %s by *%s*" repository.url repository.full_name number html_url title action sender.login) in { @@ -89,7 +89,7 @@ let generate_pr_review_notification notification = in let summary = Some - (sprintf "<%s|[%s]> %s <%s|%s> #%d <%s|%s>" repository.url repository.full_name sender.login review.html_url + (sprintf "<%s|[%s]> *%s* <%s|%s> #%d <%s|%s>" repository.url repository.full_name sender.login review.html_url action_str number html_url title) in { @@ -122,7 +122,7 @@ let generate_pr_review_comment_notification notification = in let summary = Some - (sprintf "<%s|[%s]> %s %s on #%d <%s|%s>" repository.url repository.full_name sender.login action_str number + (sprintf "<%s|[%s]> *%s* %s on #%d <%s|%s>" repository.url repository.full_name sender.login action_str number html_url title) in let file = @@ -164,7 +164,7 @@ let generate_issue_notification notification = in let summary = Some - (sprintf "<%s|[%s]> Issue #%d <%s|%s> %s by %s" repository.url repository.full_name number html_url title action + (sprintf "<%s|[%s]> Issue #%d <%s|%s> %s by *%s*" repository.url repository.full_name number html_url title action sender.login) in { @@ -198,7 +198,7 @@ let generate_issue_comment_notification notification = in let summary = Some - (sprintf "<%s|[%s]> %s <%s|%s> on #%d <%s|%s>" repository.url repository.full_name sender.login comment.html_url + (sprintf "<%s|[%s]> *%s* <%s|%s> on #%d <%s|%s>" repository.url repository.full_name sender.login comment.html_url action_str number issue.html_url title) in { @@ -338,7 +338,7 @@ let generate_commit_comment_notification cfg notification = in let summary = Some - (sprintf "<%s|[%s]> %s commented on `<%s|%s>` %s" repository.url repository.full_name sender.login + (sprintf "<%s|[%s]> *%s* commented on `<%s|%s>` %s" repository.url repository.full_name sender.login comment.html_url (git_short_sha_hash commit_id) (first_line commit.message)) in let path = From 1d12c2a23309699f09dc749d1fc6ba39e3e59598 Mon Sep 17 00:00:00 2001 From: ip Date: Thu, 1 Oct 2020 17:18:38 -0400 Subject: [PATCH 31/39] tests: promote --- test/slack_payloads.expected | 104 +++++++++++++++++------------------ 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/test/slack_payloads.expected b/test/slack_payloads.expected index f2c64614..f2d2d4ce 100644 --- a/test/slack_payloads.expected +++ b/test/slack_payloads.expected @@ -4,11 +4,11 @@ will notify #all-push-events "attachments": [ { "fallback": - " xinyuluo commented on `` Update README.md", + " *xinyuluo* commented on `` Update README.md", "mrkdwn_in": [ "pretext", "text" ], "color": "#ccc", "pretext": - " xinyuluo commented on `` Update README.md", + " *xinyuluo* commented on `` Update README.md", "text": "comment here", "footer": "New comment by xinyuluo in " @@ -21,11 +21,11 @@ will notify #all-push-events "attachments": [ { "fallback": - " xinyuluo commented on `` add new line at EOF", + " *xinyuluo* commented on `` add new line at EOF", "mrkdwn_in": [ "pretext", "text" ], "color": "#ccc", "pretext": - " xinyuluo commented on `` add new line at EOF", + " *xinyuluo* commented on `` add new line at EOF", "text": "add a commit comment" } ] @@ -36,11 +36,11 @@ will notify #a1 "attachments": [ { "fallback": - " xinyuluo commented on `` Add files via upload", + " *xinyuluo* commented on `` Add files via upload", "mrkdwn_in": [ "pretext", "text" ], "color": "#ccc", "pretext": - " xinyuluo commented on `` Add files via upload", + " *xinyuluo* commented on `` Add files via upload", "text": "this is a test for general commit comment with multiple paths" } ] @@ -50,11 +50,11 @@ will notify #all-push-events "attachments": [ { "fallback": - " xinyuluo commented on `` Add files via upload", + " *xinyuluo* commented on `` Add files via upload", "mrkdwn_in": [ "pretext", "text" ], "color": "#ccc", "pretext": - " xinyuluo commented on `` Add files via upload", + " *xinyuluo* commented on `` Add files via upload", "text": "this is a test for general commit comment with multiple paths" } ] @@ -65,11 +65,11 @@ will notify #all-push-events "attachments": [ { "fallback": - " xinyuluo commented on `` add new line at EOF", + " *xinyuluo* commented on `` add new line at EOF", "mrkdwn_in": [ "pretext", "text" ], "color": "#ccc", "pretext": - " xinyuluo commented on `` add new line at EOF", + " *xinyuluo* commented on `` add new line at EOF", "text": "this can be written shorter:\n`\n| Some serpDomain -> Domain.is_subdomain serpDomain domain\n| None -> false`", "footer": @@ -83,11 +83,11 @@ will notify #all-push-events "attachments": [ { "fallback": - " xinyuluo commented on `` add new line at EOF", + " *xinyuluo* commented on `` add new line at EOF", "mrkdwn_in": [ "pretext", "text" ], "color": "#ccc", "pretext": - " xinyuluo commented on `` add new line at EOF", + " *xinyuluo* commented on `` add new line at EOF", "text": "*bold*, _italic_\n\n> blockquote\n\n\n- list-element 1\n- list-element 2\n - list-element2.1\n - \n\n\n```\n
html block
\n```\n```\nStdio.printf \"hello ocaml\"\n```", "footer": @@ -101,11 +101,11 @@ will notify #all-push-events "attachments": [ { "fallback": - " xinyuluo commented on `` make changes in 2 files", + " *xinyuluo* commented on `` make changes in 2 files", "mrkdwn_in": [ "pretext", "text" ], "color": "#ccc", "pretext": - " xinyuluo commented on `` make changes in 2 files", + " *xinyuluo* commented on `` make changes in 2 files", "text": "make a comment" } ] @@ -116,11 +116,11 @@ will notify #all-push-events "attachments": [ { "fallback": - " xinyuluo commented on `` add new line at EOF", + " *xinyuluo* commented on `` add new line at EOF", "mrkdwn_in": [ "pretext", "text" ], "color": "#ccc", "pretext": - " xinyuluo commented on `` add new line at EOF", + " *xinyuluo* commented on `` add new line at EOF", "text": "add a single comment on the code in a commit", "footer": "New comment by xinyuluo in " @@ -133,11 +133,11 @@ will notify #default "attachments": [ { "fallback": - " xinyuluo on #5 ", + " *xinyuluo* on #5 ", "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " xinyuluo on #5 ", + " *xinyuluo* on #5 ", "text": "handle issue comment" } ] @@ -148,11 +148,11 @@ will notify #a1-bot "attachments": [ { "fallback": - " xinyuluo on #4 ", + " *xinyuluo* on #4 ", "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " xinyuluo on #4 ", + " *xinyuluo* on #4 ", "text": "comment example" } ] @@ -162,11 +162,11 @@ will notify #backend "attachments": [ { "fallback": - " xinyuluo on #4 ", + " *xinyuluo* on #4 ", "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " xinyuluo on #4 ", + " *xinyuluo* on #4 ", "text": "comment example" } ] @@ -178,11 +178,11 @@ will notify #frontend-bot "attachments": [ { "fallback": - " yasunariw on #6168 ", + " *yasunariw* on #6168 ", "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " yasunariw on #6168 ", + " *yasunariw* on #6168 ", "text": "> I do not understand this\n\n\nDo you have access to the logs on the website?" } @@ -195,11 +195,11 @@ will notify #backend "attachments": [ { "fallback": - " Issue #6 closed by xinyuluo", + " Issue #6 closed by *xinyuluo*", "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " Issue #6 closed by xinyuluo" + " Issue #6 closed by *xinyuluo*" } ] } @@ -209,11 +209,11 @@ will notify #backend "attachments": [ { "fallback": - " Issue #6 labeled by xinyuluo", + " Issue #6 labeled by *xinyuluo*", "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " Issue #6 labeled by xinyuluo", + " Issue #6 labeled by *xinyuluo*", "text": "Labels: backend" } ] @@ -224,11 +224,11 @@ will notify #default "attachments": [ { "fallback": - " Issue #6 opened by xinyuluo", + " Issue #6 opened by *xinyuluo*", "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " Issue #6 opened by xinyuluo", + " Issue #6 opened by *xinyuluo*", "text": "consider multiple cases including labeled/opened/closed/reopened" } @@ -240,11 +240,11 @@ will notify #backend "attachments": [ { "fallback": - " Issue #6 reopened by xinyuluo", + " Issue #6 reopened by *xinyuluo*", "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " Issue #6 reopened by xinyuluo" + " Issue #6 reopened by *xinyuluo*" } ] } @@ -254,11 +254,11 @@ will notify #a1-bot "attachments": [ { "fallback": - " Pull request #8 closed by xinyuluo", + " Pull request #8 closed by *xinyuluo*", "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " Pull request #8 closed by xinyuluo" + " Pull request #8 closed by *xinyuluo*" } ] } @@ -268,11 +268,11 @@ will notify #default "attachments": [ { "fallback": - " Pull request #2 closed by xinyuluo", + " Pull request #2 closed by *xinyuluo*", "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " Pull request #2 closed by xinyuluo" + " Pull request #2 closed by *xinyuluo*" } ] } @@ -282,11 +282,11 @@ will notify #a1-bot "attachments": [ { "fallback": - " Pull request #4 labeled by xinyuluo", + " Pull request #4 labeled by *xinyuluo*", "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " Pull request #4 labeled by xinyuluo", + " Pull request #4 labeled by *xinyuluo*", "text": "Labels: a1, backend" } ] @@ -296,11 +296,11 @@ will notify #backend "attachments": [ { "fallback": - " Pull request #4 labeled by xinyuluo", + " Pull request #4 labeled by *xinyuluo*", "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " Pull request #4 labeled by xinyuluo", + " Pull request #4 labeled by *xinyuluo*", "text": "Labels: a1, backend" } ] @@ -311,11 +311,11 @@ will notify #frontend-bot "attachments": [ { "fallback": - " Pull request #3 opened by xinyuluo", + " Pull request #3 opened by *xinyuluo*", "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " Pull request #3 opened by xinyuluo", + " Pull request #3 opened by *xinyuluo*", "text": "" } ] @@ -327,11 +327,11 @@ will notify #default "attachments": [ { "fallback": - " Khady #6 ", + " *Khady* #6 ", "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " Khady #6 ", + " *Khady* #6 ", "text": "approve review" } ] @@ -342,11 +342,11 @@ will notify #frontend-bot "attachments": [ { "fallback": - " xinyuluo #3 ", + " *xinyuluo* #3 ", "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " xinyuluo #3 ", + " *xinyuluo* #3 ", "text": "submit a PR review" } ] @@ -359,11 +359,11 @@ will notify #default "attachments": [ { "fallback": - " Khady #6 ", + " *Khady* #6 ", "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " Khady #6 ", + " *Khady* #6 ", "text": "request changes review" } ] @@ -376,11 +376,11 @@ will notify #a1-bot "attachments": [ { "fallback": - " xinyuluo commented on #4 ", + " *xinyuluo* commented on #4 ", "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " xinyuluo commented on #4 ", + " *xinyuluo* commented on #4 ", "text": "PR review comment example", "footer": "New comment by xinyuluo in " @@ -392,11 +392,11 @@ will notify #backend "attachments": [ { "fallback": - " xinyuluo commented on #4 ", + " *xinyuluo* commented on #4 ", "mrkdwn_in": [ "text" ], "color": "#ccc", "pretext": - " xinyuluo commented on #4 ", + " *xinyuluo* commented on #4 ", "text": "PR review comment example", "footer": "New comment by xinyuluo in " From 3d14daa2eada9dd2a45856d75b12c4eb37d2a45e Mon Sep 17 00:00:00 2001 From: ip Date: Fri, 23 Oct 2020 17:48:59 -0400 Subject: [PATCH 32/39] minor --- lib/action.ml | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index aa7a280c..ceaff8e2 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -110,8 +110,7 @@ let partition_push cfg n = |> List.concat in let prefix_chans = - let chans = List.map rules ~f:(fun (rule : prefix_rule) -> rule.chan) in - let chans = Option.value_map cfg.prefix_rules.default ~default:chans ~f:(fun default -> default :: chans) in + let chans = Option.to_list cfg.prefix_rules.default @ List.map rules ~f:(fun (rule : prefix_rule) -> rule.chan) in List.dedup_and_sort chans ~compare:String.compare in List.filter_map prefix_chans ~f:(fun chan -> @@ -127,7 +126,7 @@ let filter_label rules label = | true -> Some rule.chan) let partition_label cfg labels = - let default = Option.value_map cfg.label_rules.default ~default:[] ~f:(fun webhook -> [ webhook ]) in + let default = Option.to_list cfg.label_rules.default in match labels with | [] -> default | labels -> @@ -181,10 +180,7 @@ let partition_commit cfg files = let names = List.map ~f:(fun f -> f.filename) files in match unique_chans_of_files cfg.prefix_rules.rules names with | _ :: _ as xs -> xs - | [] -> - match cfg.prefix_rules.default with - | Some webhook -> [ webhook ] - | None -> [] + | [] -> Option.to_list cfg.prefix_rules.default let hide_cancelled (notification : status_notification) cfg = let find_cancelled status_state = @@ -221,9 +217,7 @@ let partition_status (ctx : Context.t) (n : status_notification) = let cfg = ctx.cfg in let get_commit_info () = match%lwt Github.generate_query_commmit cfg ~url:n.commit.url ~sha:n.commit.sha with - | None -> - let default = Option.value_map cfg.prefix_rules.default ~default:[] ~f:(fun webhook -> [ webhook ]) in - Lwt.return default + | None -> Lwt.return @@ Option.to_list cfg.prefix_rules.default | Some commit -> match List.exists n.branches ~f:(fun { name } -> is_main_merge_message ~msg:commit.commit.message ~branch:name cfg) @@ -257,7 +251,7 @@ let partition_status (ctx : Context.t) (n : status_notification) = res let partition_commit_comment cfg n = - let default = Option.value_map cfg.prefix_rules.default ~default:[] ~f:(fun webhook -> [ webhook ]) in + let default = Option.to_list cfg.prefix_rules.default in match n.comment.path with | None -> ( match%lwt Github.generate_commit_from_commit_comment cfg n with From 2edb227f7be3ff2d6ecd63d2c3a73b545dfd1569 Mon Sep 17 00:00:00 2001 From: ip Date: Fri, 23 Oct 2020 18:13:04 -0400 Subject: [PATCH 33/39] non-main branch build notifications go to default channel only (ref #81) --- lib/action.ml | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index ceaff8e2..daf2baff 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -216,16 +216,28 @@ let hide_success (n : status_notification) (ctx : Context.t) = let partition_status (ctx : Context.t) (n : status_notification) = let cfg = ctx.cfg in let get_commit_info () = - match%lwt Github.generate_query_commmit cfg ~url:n.commit.url ~sha:n.commit.sha with - | None -> Lwt.return @@ Option.to_list cfg.prefix_rules.default - | Some commit -> - match - List.exists n.branches ~f:(fun { name } -> is_main_merge_message ~msg:commit.commit.message ~branch:name cfg) - with + let default () = Lwt.return @@ Option.to_list cfg.prefix_rules.default in + match cfg.main_branch_name with + | None -> default () + | Some main_branch_name -> + (* non-main branch build notifications go to default channel to reduce spam in topic channels *) + match List.exists n.branches ~f:(fun { name } -> String.equal name main_branch_name) with + | false -> default () | true -> - log#info "main branch merge, ignoring status event %s: %s" n.context (first_line commit.commit.message); - Lwt.return [] - | false -> Lwt.return (partition_commit cfg commit.files) + ( match%lwt Github.generate_query_commmit cfg ~url:n.commit.url ~sha:n.commit.sha with + | None -> default () + | Some commit -> + (* + match + List.exists n.branches ~f:(fun { name } -> is_main_merge_message ~msg:commit.commit.message ~branch:name cfg) + with + | true -> + log#info "main branch merge, ignoring status event %s: %s" n.context (first_line commit.commit.message); + Lwt.return [] + | false -> +*) + Lwt.return (partition_commit cfg commit.files) + ) in let res = match From 4137e70d883d446157d9da36a41d7ef77d74718f Mon Sep 17 00:00:00 2001 From: ip Date: Wed, 28 Oct 2020 13:30:55 -0400 Subject: [PATCH 34/39] implicit_transitive_deps false --- dune-project | 1 + lib/dune | 4 ++-- notabot.opam | 4 ++++ src/dune | 2 +- 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/dune-project b/dune-project index dad6dc0b..8ef8aff7 100644 --- a/dune-project +++ b/dune-project @@ -1,4 +1,5 @@ (lang dune 2.5) +(implicit_transitive_deps false) (formatting (enabled_for ocaml reason)) diff --git a/lib/dune b/lib/dune index 64c32c45..24fa88ac 100644 --- a/lib/dune +++ b/lib/dune @@ -1,7 +1,7 @@ (library (name lib) - (libraries curl curl.lwt nocrypto hex atdgen base stdio lwt lwt.unix uri - devkit devkit.core omd) + (libraries cstruct curl curl.lwt nocrypto hex atdgen atdgen-runtime biniou yojson base base.caml re stdio lwt lwt.unix uri + devkit omd) (preprocess (pps lwt_ppx))) diff --git a/notabot.opam b/notabot.opam index 90f414c4..4d745a83 100644 --- a/notabot.opam +++ b/notabot.opam @@ -13,15 +13,19 @@ depends: [ "dune" {>= "2.5.0"} "atdgen" "base" + "biniou" "cmdliner" + "cstruct" "devkit" "hex" "lwt" "lwt_ppx" "nocrypto" + "re" "stdio" "uri" "ocamlformat" {dev & = "0.13.0"} "omd" {< "2.0.0"} + "yojson" ] build: ["dune" "build" "-p" name] diff --git a/src/dune b/src/dune index 62680a81..b9308a38 100644 --- a/src/dune +++ b/src/dune @@ -1,5 +1,5 @@ (executable - (libraries lib cmdliner devkit devkit.core) + (libraries lib base base.caml cmdliner devkit lwt.unix stdio uri) (preprocess (pps lwt_ppx)) (public_name notabot)) From 3275d4fd3c547d1e27a9b236ecb9c2600e3a5096 Mon Sep 17 00:00:00 2001 From: Yasunari Watanabe Date: Mon, 7 Dec 2020 16:44:18 +0800 Subject: [PATCH 35/39] tests: make tests compile again with explicit transitive deps --- test/dune | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dune b/test/dune index 9511a626..beac4f7c 100644 --- a/test/dune +++ b/test/dune @@ -1,6 +1,6 @@ (executable (name test) - (libraries lib) + (libraries lib base base.caml devkit lwt.unix stdio yojson) (preprocess (pps lwt_ppx))) From 4c56de8fa42a1e60a9073625e70d4ae89e28ad26 Mon Sep 17 00:00:00 2001 From: Yasunari Watanabe Date: Mon, 7 Dec 2020 16:49:36 +0800 Subject: [PATCH 36/39] tests: update expected with change from 2edb227 --- test/slack_payloads.expected | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/slack_payloads.expected b/test/slack_payloads.expected index f2d2d4ce..1b22f7f9 100644 --- a/test/slack_payloads.expected +++ b/test/slack_payloads.expected @@ -463,7 +463,7 @@ will notify #longest-a1 } ===== file ../mock_payloads/status.cancelled_test.json ===== ===== file ../mock_payloads/status.failure_test.json ===== -will notify #all-push-events +will notify #default { "attachments": [ { @@ -487,7 +487,7 @@ will notify #all-push-events ===== file ../mock_payloads/status.pending_test.json ===== ===== file ../mock_payloads/status.state_hide_success_test.json ===== ===== file ../mock_payloads/status.success_public_repo_no_buildkite.json ===== -will notify #all-push-events +will notify #default { "attachments": [ { @@ -507,7 +507,7 @@ will notify #all-push-events ] } ===== file ../mock_payloads/status.success_test.json ===== -will notify #all-push-events +will notify #default { "attachments": [ { From 6ecc5149465a86893685f2cec82dac3f4c2a913e Mon Sep 17 00:00:00 2001 From: Yasunari Watanabe Date: Tue, 8 Dec 2020 11:11:04 +0800 Subject: [PATCH 37/39] re-add devkit.core dependency removed in 4137e70 --- lib/dune | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/dune b/lib/dune index 24fa88ac..684be1da 100644 --- a/lib/dune +++ b/lib/dune @@ -1,7 +1,7 @@ (library (name lib) - (libraries cstruct curl curl.lwt nocrypto hex atdgen atdgen-runtime biniou yojson base base.caml re stdio lwt lwt.unix uri - devkit omd) + (libraries atdgen atdgen-runtime base base.caml biniou cstruct curl curl.lwt + devkit devkit.core hex lwt lwt.unix nocrypto omd re stdio uri yojson) (preprocess (pps lwt_ppx))) From 75d47c5a770b296fe605501386a63308f0cc84d8 Mon Sep 17 00:00:00 2001 From: Yasunari Watanabe Date: Tue, 8 Dec 2020 11:34:43 +0800 Subject: [PATCH 38/39] Add devkit.core to src --- src/dune | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dune b/src/dune index b9308a38..9f35600c 100644 --- a/src/dune +++ b/src/dune @@ -1,5 +1,5 @@ (executable - (libraries lib base base.caml cmdliner devkit lwt.unix stdio uri) + (libraries lib base base.caml cmdliner devkit devkit.core lwt.unix stdio uri) (preprocess (pps lwt_ppx)) (public_name notabot)) From e7a49a1a8585997384a1b3c43e43dc2156ddad9c Mon Sep 17 00:00:00 2001 From: Yasunari Watanabe Date: Tue, 8 Dec 2020 11:37:19 +0800 Subject: [PATCH 39/39] Add devkit.core to test --- test/dune | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dune b/test/dune index beac4f7c..acb2d487 100644 --- a/test/dune +++ b/test/dune @@ -1,6 +1,6 @@ (executable (name test) - (libraries lib base base.caml devkit lwt.unix stdio yojson) + (libraries lib base base.caml devkit devkit.core lwt.unix stdio yojson) (preprocess (pps lwt_ppx)))