From 65d23a9494f88eb6a57e41eae17e3d67986b494f Mon Sep 17 00:00:00 2001 From: Corentin Leruth Date: Tue, 12 Mar 2024 12:03:32 +0100 Subject: [PATCH 1/8] fix link to pipeline when using buildkite --- lib/slack.ml | 26 ++++++++++++++++++++++++-- test/slack_payloads.expected | 8 ++++---- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/lib/slack.ml b/lib/slack.ml index 644ee0f6..4df3c6f0 100644 --- a/lib/slack.ml +++ b/lib/slack.ml @@ -272,8 +272,30 @@ let generate_status_notification (cfg : Config_t.config) (notification : status_ sprintf "<%s|[%s]> CI Build Status notification: %s" repository.url repository.full_name state_info (* in case the CI run is not using buildkite *) | Some target_url -> - sprintf "<%s|[%s]> CI Build Status notification for <%s|%s>: %s" repository.url repository.full_name target_url - context state_info + (* try to compute the pipeline_url based on the `context` (e.g. buildkite/pipeline) and the `target_url` (link to the build) *) + let pipeline_url = + let pipeline_name = context |> String.split ~on:'/' |> List.last in + match pipeline_name with + | None -> None + | Some pipeline_name -> + let rec loop acc els = + match els with + | [] -> None + | s :: _ when String.(s = pipeline_name) -> + let url = pipeline_name :: acc |> List.rev |> String.concat ~sep:"/" in + Some url + | s :: els -> loop (s :: acc) els + in + loop [] (String.split ~on:'/' target_url) + in + ( match pipeline_url with + | None -> + sprintf "<%s|[%s]> CI Build Status notification for %s: %s" repository.url repository.full_name context + state_info + | Some pipeline_url -> + sprintf "<%s|[%s]> CI Build Status notification for <%s|%s>: %s" repository.url repository.full_name + pipeline_url context state_info + ) in let msg = String.concat ~sep:"\n" @@ List.concat [ commit_info; branches_info ] in let attachment = diff --git a/test/slack_payloads.expected b/test/slack_payloads.expected index 5e1e2672..b2bc7eb3 100644 --- a/test/slack_payloads.expected +++ b/test/slack_payloads.expected @@ -434,7 +434,7 @@ will notify #longest-a1 will notify #id[slack_mail@example.com] { "channel": "id[slack_mail@example.com]", - "text": " CI Build Status notification for : failure", + "text": " CI Build Status notification for : failure", "attachments": [ { "fallback": null, @@ -453,7 +453,7 @@ will notify #id[slack_mail@example.com] will notify #default { "channel": "default", - "text": " CI Build Status notification for : failure", + "text": " CI Build Status notification for : failure", "attachments": [ { "fallback": null, @@ -496,7 +496,7 @@ will notify #default will notify #all-push-events { "channel": "all-push-events", - "text": " CI Build Status notification for : success", + "text": " CI Build Status notification for : success", "attachments": [ { "fallback": null, @@ -516,7 +516,7 @@ will notify #all-push-events will notify #default { "channel": "default", - "text": " CI Build Status notification for : success", + "text": " CI Build Status notification for : success", "attachments": [ { "fallback": null, From 90eee3d20548379015da7ae5d1e139e73b71558b Mon Sep 17 00:00:00 2001 From: Corentin Leruth Date: Tue, 12 Mar 2024 14:31:50 +0100 Subject: [PATCH 2/8] update notification title --- lib/slack.ml | 33 +++++++++++++++------------------ test/slack_payloads.expected | 10 +++++----- 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/lib/slack.ml b/lib/slack.ml index 4df3c6f0..af9f3f87 100644 --- a/lib/slack.ml +++ b/lib/slack.ml @@ -227,17 +227,6 @@ let generate_status_notification (cfg : Config_t.config) (notification : status_ 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 - let state_info = - match state with - | Success -> "success" - | Failure -> "failure" - | Error -> "error" - | _ -> - invalid_arg - (sprintf "Monorobot doesn't know how to generate notification for the unexpected event %s of %s" - (string_of_status_state state) sha - ) - in let color_info = match state with | Success -> "good" @@ -267,9 +256,21 @@ let generate_status_notification (cfg : Config_t.config) (notification : status_ ] in let summary = + let state_info = + match state with + | Success -> "succeeded" + | Failure -> "failed" + | Error -> "error" + | _ -> + invalid_arg + (sprintf "Monorobot doesn't know how to generate notification for the unexpected event %s of %s" + (string_of_status_state state) sha + ) + in + let commit_message = first_line message in match target_url with | None -> - sprintf "<%s|[%s]> CI Build Status notification: %s" repository.url repository.full_name state_info + sprintf "<%s|[%s]>: Build %s for \"%s\"" repository.url repository.full_name state_info commit_message (* in case the CI run is not using buildkite *) | Some target_url -> (* try to compute the pipeline_url based on the `context` (e.g. buildkite/pipeline) and the `target_url` (link to the build) *) @@ -289,12 +290,8 @@ let generate_status_notification (cfg : Config_t.config) (notification : status_ loop [] (String.split ~on:'/' target_url) in ( match pipeline_url with - | None -> - sprintf "<%s|[%s]> CI Build Status notification for %s: %s" repository.url repository.full_name context - state_info - | Some pipeline_url -> - sprintf "<%s|[%s]> CI Build Status notification for <%s|%s>: %s" repository.url repository.full_name - pipeline_url context state_info + | None -> sprintf "%s: Build %s for \"%s\"" context state_info commit_message + | Some pipeline_url -> sprintf "<%s|[%s]>: Build %s for \"%s\"" pipeline_url context state_info commit_message ) in let msg = String.concat ~sep:"\n" @@ List.concat [ commit_info; branches_info ] in diff --git a/test/slack_payloads.expected b/test/slack_payloads.expected index b2bc7eb3..d1837d25 100644 --- a/test/slack_payloads.expected +++ b/test/slack_payloads.expected @@ -434,7 +434,7 @@ will notify #longest-a1 will notify #id[slack_mail@example.com] { "channel": "id[slack_mail@example.com]", - "text": " CI Build Status notification for : failure", + "text": ": Build failed for \"Update README.md\"", "attachments": [ { "fallback": null, @@ -453,7 +453,7 @@ will notify #id[slack_mail@example.com] will notify #default { "channel": "default", - "text": " CI Build Status notification for : failure", + "text": ": Build failed for \"Update README.md\"", "attachments": [ { "fallback": null, @@ -477,7 +477,7 @@ will notify #default will notify #default { "channel": "default", - "text": " CI Build Status notification: success", + "text": ": Build succeeded for \"Initial commit\"", "attachments": [ { "fallback": null, @@ -496,7 +496,7 @@ will notify #default will notify #all-push-events { "channel": "all-push-events", - "text": " CI Build Status notification for : success", + "text": ": Build succeeded for \"Update README.md\"", "attachments": [ { "fallback": null, @@ -516,7 +516,7 @@ will notify #all-push-events will notify #default { "channel": "default", - "text": " CI Build Status notification for : success", + "text": ": Build succeeded for \"Update README.md\"", "attachments": [ { "fallback": null, From 502475650b620cab2d049a3b828e2f0a3b188135 Mon Sep 17 00:00:00 2001 From: Corentin Leruth Date: Fri, 15 Mar 2024 14:57:05 +0100 Subject: [PATCH 3/8] simplify code to construct pipeline url --- lib/slack.ml | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/lib/slack.ml b/lib/slack.ml index f9b8f0af..ab002910 100644 --- a/lib/slack.ml +++ b/lib/slack.ml @@ -287,21 +287,15 @@ let generate_status_notification (cfg : Config_t.config) (notification : status_ sprintf "<%s|[%s]>: Build %s for \"%s\"" repository.url repository.full_name state_info commit_message (* in case the CI run is not using buildkite *) | Some target_url -> - (* try to compute the pipeline_url based on the `context` (e.g. buildkite/pipeline) and the `target_url` (link to the build) *) + (* Keep only the portion of the url before /builds/... *) let pipeline_url = - let pipeline_name = context |> String.split ~on:'/' |> List.last in - match pipeline_name with - | None -> None - | Some pipeline_name -> - let rec loop acc els = - match els with - | [] -> None - | s :: _ when String.(s = pipeline_name) -> - let url = pipeline_name :: acc |> List.rev |> String.concat ~sep:"/" in - Some url - | s :: els -> loop (s :: acc) els - in - loop [] (String.split ~on:'/' target_url) + let rec loop = function + | [] -> None + | "builds" :: l -> Some (List.rev l |> String.concat ~sep:"/") + | _ :: l -> loop l + in + let url_parts = target_url |> String.split ~on:'/' |> List.rev in + loop url_parts in ( match pipeline_url with | None -> sprintf "%s: Build %s for \"%s\"" context state_info commit_message From 74fe71c3a98a4fd59565287b4dd78332008e906b Mon Sep 17 00:00:00 2001 From: Corentin Leruth Date: Fri, 15 Mar 2024 15:26:11 +0100 Subject: [PATCH 4/8] apply changes only if buildkite notification --- lib/slack.ml | 35 ++++++++++++++++++++--------------- test/slack_payloads.expected | 2 +- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/lib/slack.ml b/lib/slack.ml index ab002910..772db532 100644 --- a/lib/slack.ml +++ b/lib/slack.ml @@ -283,23 +283,28 @@ let generate_status_notification (cfg : Config_t.config) (notification : status_ in let commit_message = first_line message in match target_url with - | None -> - sprintf "<%s|[%s]>: Build %s for \"%s\"" repository.url repository.full_name state_info commit_message - (* in case the CI run is not using buildkite *) + | None -> sprintf "<%s|[%s]> CI Build Status notification: %s" repository.url repository.full_name state_info | Some target_url -> - (* Keep only the portion of the url before /builds/... *) - let pipeline_url = - let rec loop = function - | [] -> None - | "builds" :: l -> Some (List.rev l |> String.concat ~sep:"/") - | _ :: l -> loop l - in - let url_parts = target_url |> String.split ~on:'/' |> List.rev in - loop url_parts + let is_buildkite = String.is_prefix context ~prefix:"buildkite" in + let default_summary = + sprintf "<%s|[%s]> CI Build Status notification for <%s|%s>: %s" repository.url repository.full_name target_url + context state_info in - ( match pipeline_url with - | None -> sprintf "%s: Build %s for \"%s\"" context state_info commit_message - | Some pipeline_url -> sprintf "<%s|[%s]>: Build %s for \"%s\"" pipeline_url context state_info commit_message + if not is_buildkite then default_summary + else ( + (* Keep only the portion of the url before /builds/... *) + let pipeline_url = + let rec loop = function + | [] -> None + | "builds" :: l -> Some (List.rev l |> String.concat ~sep:"/") + | _ :: l -> loop l + in + let url_parts = target_url |> String.split ~on:'/' |> List.rev in + loop url_parts + in + match pipeline_url with + | None -> default_summary + | Some pipeline_url -> sprintf "<%s|[%s]>: Build %s for \"%s\"" pipeline_url context state_info commit_message ) in let msg = String.concat ~sep:"\n" @@ List.concat [ commit_info; branches_info ] in diff --git a/test/slack_payloads.expected b/test/slack_payloads.expected index 883ef0b4..46e1c9dc 100644 --- a/test/slack_payloads.expected +++ b/test/slack_payloads.expected @@ -477,7 +477,7 @@ will notify #default will notify #default { "channel": "default", - "text": ": Build succeeded for \"Initial commit\"", + "text": " CI Build Status notification: succeeded", "attachments": [ { "fallback": null, From 77c6d418f2362c6acc120b2db299a03a778e5749 Mon Sep 17 00:00:00 2001 From: Corentin Leruth Date: Fri, 15 Mar 2024 15:28:58 +0100 Subject: [PATCH 5/8] apply description changes only for buildkite notif --- lib/slack.ml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/slack.ml b/lib/slack.ml index 772db532..d3223ae8 100644 --- a/lib/slack.ml +++ b/lib/slack.ml @@ -229,6 +229,7 @@ let generate_status_notification (cfg : Config_t.config) (notification : status_ 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 + let is_buildkite = String.is_prefix context ~prefix:"buildkite" in let color_info = match state with | Success -> "good" @@ -241,6 +242,7 @@ let generate_status_notification (cfg : Config_t.config) (notification : status_ let text = match target_url with | None -> s + | Some target_url when not is_buildkite -> sprintf "*Description*: %s." target_url | Some target_url -> (* Specific to buildkite *) match Re2.find_submatches_exn buildkite_description_re s with @@ -285,7 +287,6 @@ let generate_status_notification (cfg : Config_t.config) (notification : status_ match target_url with | None -> sprintf "<%s|[%s]> CI Build Status notification: %s" repository.url repository.full_name state_info | Some target_url -> - let is_buildkite = String.is_prefix context ~prefix:"buildkite" in let default_summary = sprintf "<%s|[%s]> CI Build Status notification for <%s|%s>: %s" repository.url repository.full_name target_url context state_info From 08107a33b75378289156e3af80e8ae305c3f0916 Mon Sep 17 00:00:00 2001 From: Corentin Leruth Date: Tue, 26 Mar 2024 08:06:19 +0100 Subject: [PATCH 6/8] simplify pipeline_url computation --- lib/slack.ml | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/slack.ml b/lib/slack.ml index 69271847..b3f3ee1f 100644 --- a/lib/slack.ml +++ b/lib/slack.ml @@ -290,13 +290,7 @@ let generate_status_notification (cfg : Config_t.config) (notification : status_ else ( (* Keep only the portion of the url before /builds/... *) let pipeline_url = - let rec loop = function - | [] -> None - | "builds" :: l -> Some (List.rev l |> String.concat "/") - | _ :: l -> loop l - in - let url_parts = target_url |> String.split_on_char '/' |> List.rev in - loop url_parts + try Some (ExtLib.String.split target_url "/builds" |> fst) with ExtLib.Invalid_string -> None in match pipeline_url with | None -> default_summary From 9498f45723fe87956490e53b8cdfba2e21fc8a88 Mon Sep 17 00:00:00 2001 From: Corentin Leruth Date: Tue, 26 Mar 2024 08:21:40 +0100 Subject: [PATCH 7/8] fix description when not is_buildkite --- lib/slack.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/slack.ml b/lib/slack.ml index b3f3ee1f..7e1bc2b3 100644 --- a/lib/slack.ml +++ b/lib/slack.ml @@ -239,7 +239,7 @@ let generate_status_notification (cfg : Config_t.config) (notification : status_ let text = match target_url with | None -> s - | Some target_url when not is_buildkite -> sprintf "*Description*: %s." target_url + | Some _ when not is_buildkite -> s | Some target_url -> (* Specific to buildkite *) match Re2.find_submatches_exn buildkite_description_re s with From 7f51de2341e0bd2f0f268def58c1ea5326bc1457 Mon Sep 17 00:00:00 2001 From: Corentin Leruth Date: Tue, 26 Mar 2024 08:33:01 +0100 Subject: [PATCH 8/8] use pattern matching for pipeline computation --- lib/slack.ml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/slack.ml b/lib/slack.ml index 7e1bc2b3..adb64232 100644 --- a/lib/slack.ml +++ b/lib/slack.ml @@ -290,7 +290,10 @@ let generate_status_notification (cfg : Config_t.config) (notification : status_ else ( (* Keep only the portion of the url before /builds/... *) let pipeline_url = - try Some (ExtLib.String.split target_url "/builds" |> fst) with ExtLib.Invalid_string -> None + match String.split_on_char '/' target_url with + | "https:" :: "" :: "buildkite.com" :: "org" :: pipeline :: "builds" :: _ -> + Some (Printf.sprintf "https://buildkite.com/org/%s" pipeline) + | _ -> None in match pipeline_url with | None -> default_summary