From 8c0b59672b14f3aa13afd8bd775e850a705d3c5d Mon Sep 17 00:00:00 2001
From: Phil Saxton <phillip.saxton@tqtezos.com>
Date: Wed, 11 Jan 2023 13:46:39 -0500
Subject: [PATCH 1/8] Main/Bot_actions: Add dry-run and more reporting.

Add the --dry-run option to the cli wich trigers the reporting_only
option. Add reporing_only to post/edit comment/thread functions.
---
 src/app/merbocop.ml    | 17 ++++++---
 src/lib/bot_actions.ml | 79 ++++++++++++++++++++++++++++--------------
 2 files changed, 66 insertions(+), 30 deletions(-)

diff --git a/src/app/merbocop.ml b/src/app/merbocop.ml
index 62a6f9e..cb2b924 100644
--- a/src/app/merbocop.ml
+++ b/src/app/merbocop.ml
@@ -259,7 +259,7 @@ let post_comments (self_id : int option) (token : string option)
     (mr_list : (string * Merge_request.mr_result) list)
     (retry_options : Web_api.Retry_options.t) (errors : string list ref)
     (comment_on_self : bool) (warnings : Warnings.t) (do_edit_existing : bool)
-    (require_resolve_thread : bool) =
+    (require_resolve_thread : bool) (reporting_only : bool) =
   if comment_on_self then
     List.iter mr_list ~f:(function
       | _, Error (`Mr_not_found _) -> (* Already counted *) ()
@@ -280,7 +280,7 @@ let post_comments (self_id : int option) (token : string option)
               ~user_id:
                 (Option.value_exn
                    ~message:"Commenting requires the --self-id option." self_id )
-              ~retry_options ~warnings body_of_prev
+              ~retry_options ~warnings ~reporting_only body_of_prev
           with
           | Ok (`Added res) | Ok (`Edited res) -> (
               say "Ok, done %s/%a %s." project Merge_request.pp_quick (Ok mr)
@@ -456,6 +456,14 @@ let wip_status () =
     const (fun w -> `Wip_status w)
     $ value (opt (enum choices) `Any (info ["wip-status-filter"] ~doc)))
 
+let reporting_only () =
+  Arg.(
+    const (fun s -> `Reporting_only s)
+    $ value
+        (flag
+           (info ["dry-run"]
+              ~doc:"Generate then full report without executing api calls." ) ))
+
 let () =
   let multi_inspect_cmd =
     ( const
@@ -480,6 +488,7 @@ let () =
           (`Post_warnings post_warnings_to)
           (`Assign_reviewers assign_reviewers)
           (`Require_resolve_thread resolve_required)
+          (`Reporting_only report_only)
         ->
           Option.iter opt_load_path ~f:(fun path -> Web_api.load_cache ~path) ;
           Caml.at_exit (fun () ->
@@ -513,7 +522,7 @@ let () =
                      Some (Fmt.str "MR-not-found: %s/%s: %a" proj ci Exn.pp e) ) ;
            if comment_on_self then
              post_comments self token mr_list retry errors comment_on_self
-               job_warnings do_edit_existing resolve_required ;
+               job_warnings do_edit_existing resolve_required report_only ;
            label_mr self token retry mr_list errors approve_doc_only doc_label
              approved_label ;
            approve_mr self token retry mr_list errors approve_doc_only ;
@@ -536,6 +545,6 @@ let () =
       $ no_comment_editing () $ write_body () $ load_cache () $ dump_cache ()
       $ set_doc_label () $ set_approved_label () $ approve_doc_only_mr ()
       $ retry_options () $ no_retry () $ post_warnings_to ()
-      $ Reviewers.cli_term () $ Thread.cli_term ()
+      $ Reviewers.cli_term () $ Thread.cli_term () $ reporting_only ()
     , info "full-inspection" ~doc:"Do full inspection on a bunch of MRs" ) in
   exit (eval_choice (help : unit Term.t * _) [multi_inspect_cmd])
diff --git a/src/lib/bot_actions.ml b/src/lib/bot_actions.ml
index 1601db5..148e744 100644
--- a/src/lib/bot_actions.ml
+++ b/src/lib/bot_actions.ml
@@ -7,7 +7,7 @@ open! Base
 let post_or_edit_mr_comment ?(do_edit_existing = true) ?(token : string option)
     ~(mr_result : Merge_request.mr_result) ~(user_id : int)
     ~(retry_options : Web_api.Retry_options.t) ~(warnings : Warnings.t)
-    body_of_prev =
+    ~(reporting_only : bool) body_of_prev =
   let open Merge_request in
   let my_comment =
     match mr_result with
@@ -35,16 +35,20 @@ let post_or_edit_mr_comment ?(do_edit_existing = true) ?(token : string option)
     let open Jq in
     try get_dict result |> get_field "id" |> get_int
     with e ->
-      Debug.dbg "Merge_request.post_or_edit_mr_comment \n\n\n%s, %s\n\n"
+      Debug.dbg "Bot_actions.post_or_edit_mr_comment \n\n\n%s, %s\n\n"
         (Exn.to_string_mach e)
         (Ezjsonm.value_to_string result) ;
       raise e in
+  let report_json id = Jq.(dict [("id", int id); ("reporting", `String "OK")]) in
   match (my_comment, mr_result) with
   | None, Ok {id; project; reports; _} ->
+      (* If no previous comment, post new one. *)
       let add_result =
-        let body = body_of_prev None in
-        Comment.add_merge_request_note ?token ~project:project.id ~mr:id
-          ~retry_options body in
+        if reporting_only then report_json 000000
+        else
+          let body = body_of_prev None in
+          Comment.add_merge_request_note ?token ~project:project.id ~mr:id
+            ~retry_options body in
       let new_id = note_id add_result in
       Reporting.add_conditional reports
         (Fmt.str
@@ -54,15 +58,22 @@ let post_or_edit_mr_comment ?(do_edit_existing = true) ?(token : string option)
       Ok (`Added add_result)
   | Some (i, prev_body), Ok mr -> (
       let body = body_of_prev (Some prev_body) in
+      let report_res = report_json i in
       let edit_result =
-        Comment.edit_merge_request_note ?token ~project:mr.project.id ~mr:mr.id
-          ~retry_options ~note:i body in
+        if reporting_only then report_res
+        else
+          Comment.edit_merge_request_note ?token ~project:mr.project.id
+            ~mr:mr.id ~retry_options ~note:i body in
       match edit_result with
       | `Float 500. ->
           let address =
-            Comment.add_merge_request_note ?token ~project:mr.project.id
-              ~mr:mr.id ~retry_options body in
+            if reporting_only then report_res
+            else
+              Comment.add_merge_request_note ?token ~project:mr.project.id
+                ~mr:mr.id ~retry_options body in
           let new_id = note_id address in
+          Reporting.add_conditional mr.reports
+            (Fmt.str "Failed to edit comment with response code 500.") ;
           Warnings.add ~warnings ~mr
             ~s:
               (Fmt.str
@@ -94,7 +105,7 @@ let post_or_edit_mr_comment ?(do_edit_existing = true) ?(token : string option)
 let post_or_edit_mr_thread ?(do_edit_existing = true) ?(token : string option)
     ~(mr_result : Merge_request.mr_result) ~(user_id : int)
     ~(retry_options : Web_api.Retry_options.t) ~(warnings : Warnings.t)
-    body_of_prev =
+    ~(reporting_only : bool) body_of_prev =
   let open Merge_request in
   let match_comment (c : Comment.t) =
     match c with
@@ -132,16 +143,21 @@ let post_or_edit_mr_thread ?(do_edit_existing = true) ?(token : string option)
     try get_dict result |> get_field "id" |> get_int
     with e ->
       Debug.dbg
-        "Merge_request.post_or_edit_mr_thread note_id error \n\n\n%s, %s\n\n"
+        "Bot_actions.post_or_edit_mr_thread note_id error \n\n\n%s, %s\n\n"
         (Exn.to_string_mach e)
         (Ezjsonm.value_to_string result) ;
       raise e in
+  let report_thread id =
+    Jq.(dict [("id", string id); ("reporting", `String "OK")]) in
+  let report_note id = Jq.(dict [("id", int id); ("reporting", `String "OK")]) in
   match (my_thread, mr_result) with
   | None, Ok mr ->
       let add_result =
-        let body = body_of_prev None in
-        Thread.start_merge_request_discussion ?token ~project:mr.project.id
-          ~mr:mr.id ~retry_options body in
+        if reporting_only then report_thread "dry-run-no-thread-id"
+        else
+          let body = body_of_prev None in
+          Thread.start_merge_request_discussion ?token ~project:mr.project.id
+            ~mr:mr.id ~retry_options body in
       let new_id = thread_id add_result in
       Reporting.add_conditional mr.reports
         (Fmt.str
@@ -151,15 +167,25 @@ let post_or_edit_mr_thread ?(do_edit_existing = true) ?(token : string option)
       Ok (`Added add_result)
   | Some (id, (note, prev_body)), Ok mr -> (
       let body = body_of_prev (Some prev_body) in
+      let report_res = report_thread "dry-run-no-thread-id" in
       let edit_result =
-        Thread.edit_merge_request_discussion ?token ~project:mr.project.id
-          ~mr:mr.id ~retry_options ~discussion:id ~note body in
+        if reporting_only then report_res
+        else
+          Thread.edit_merge_request_discussion ?token ~project:mr.project.id
+            ~mr:mr.id ~retry_options ~discussion:id ~note body in
       match edit_result with
       | `Float 500. ->
-          let add =
-            Thread.start_merge_request_discussion ?token ~project:mr.project.id
-              ~mr:mr.id ~retry_options body in
-          let new_id = thread_id add in
+          let add_ress =
+            if reporting_only then report_res
+            else
+              Thread.start_merge_request_discussion ?token
+                ~project:mr.project.id ~mr:mr.id ~retry_options body in
+          let new_id = thread_id add_ress in
+          Reporting.add_conditional mr.reports
+            (Fmt.str
+               "Failed to edit a previous threat with response code 500. A new \
+                thread was posted mr:%d thread-id:%s"
+               mr.id new_id ) ;
           Warnings.add ~warnings ~mr
             ~s:
               (Fmt.str
@@ -167,10 +193,10 @@ let post_or_edit_mr_thread ?(do_edit_existing = true) ?(token : string option)
                   new thread was posted mr:%d thread-id:%s"
                  mr.id new_id )
             () ;
-          Ok (`Added add)
+          Ok (`Added add_ress)
       | _ ->
           let new_id =
-            note_id edit_result
+            note_id (if reporting_only then report_note note else edit_result)
             (* API for Editing a descussion note returns only the note JSON info. *)
           in
           if Int.equal note new_id then
@@ -197,19 +223,20 @@ let post_or_edit_mr_comment_or_thread ?(require_resolve_thread = false)
     ?(do_edit_existing = true) ?(token : string option)
     ~(mr_result : Merge_request.mr_result) ~(user_id : int)
     ~(retry_options : Web_api.Retry_options.t) ~(warnings : Warnings.t)
-    body_of_prev =
+    ~(reporting_only : bool) body_of_prev =
   if require_resolve_thread then
     post_or_edit_mr_thread ~do_edit_existing ?token ~mr_result ~user_id
-      ~retry_options ~warnings body_of_prev
+      ~retry_options ~warnings ~reporting_only body_of_prev
   else
     post_or_edit_mr_comment ~do_edit_existing ?token ~mr_result ~user_id
-      ~retry_options ~warnings body_of_prev
+      ~retry_options ~warnings ~reporting_only body_of_prev
 
 (** [check_json message api_call] is the [api_call] : Ezjosnm.value or an
     exception. check_json m a will attempt to parse the JOSN data [a] returned
     by a call to the Gitlab API by checking the "id" field. A failure raises and
     exception with the message [m]. *)
-let check_json ?(message = "Gitlab API call") (api_call : Ezjsonm.value) =
+let check_json ?(message = "Gitlab API call") (api_call : Ezjsonm.value) : Jq.t
+    =
   let open Jq in
   try
     get_dict api_call |> get_field "id" |> get_int
-- 
GitLab


From 7d6ab05006bf3bdba4583226ece05a391b6f33f0 Mon Sep 17 00:00:00 2001
From: Phil Saxton <phillip.saxton@tqtezos.com>
Date: Wed, 11 Jan 2023 15:48:25 -0500
Subject: [PATCH 2/8] bot_actions: refactor and improve reporting.

post_or_edit_comment and post_or_edit_thread are refactored with some of
the logic moved to the Thread and Comment modules. Also added some
reporting.
---
 src/lib/bot_actions.ml | 166 +++++++++++++++++++----------------------
 src/lib/comment.ml     |  11 ++-
 src/lib/thread.ml      |  16 ++++
 3 files changed, 104 insertions(+), 89 deletions(-)

diff --git a/src/lib/bot_actions.ml b/src/lib/bot_actions.ml
index 148e744..4f9f5c8 100644
--- a/src/lib/bot_actions.ml
+++ b/src/lib/bot_actions.ml
@@ -1,36 +1,24 @@
 open! Base
 
 (** [post_or_edit_mr_comment] will post a Gitlab note (comment) [body_of_prev]
-    on merge request [mr_result] or give error "merge request not found."
-    [do_edit_existing] will attempt to edit Merbocop's preexisting note before
-    posting a new one. *)
+    on merge request [mr_result]. [do_edit_existing] will attempt to edit
+    previous comments by matching [user_id] preexisting note before posting a
+    new one. *)
 let post_or_edit_mr_comment ?(do_edit_existing = true) ?(token : string option)
     ~(mr_result : Merge_request.mr_result) ~(user_id : int)
     ~(retry_options : Web_api.Retry_options.t) ~(warnings : Warnings.t)
     ~(reporting_only : bool) body_of_prev =
   let open Merge_request in
   let my_comment =
-    match mr_result with
-    | Ok {comments= _; _} when not do_edit_existing -> None
-    | Ok {comments; _} ->
-        List.find_map
-          (Option.value_exn
-             ~message:
-               "Please provide the --access-token option to post comments."
-             comments ) ~f:(function
-          | {Comment.system= true; _} | {Comment.resolvable= true; _} -> None
-          | {Comment.json; _} ->
-              let open Jq in
-              let obj = get_dict json in
-              if
-                get_field "author" obj |> get_dict |> get_field "id" |> get_int
-                = user_id
-              then
-                Some
-                  ( get_field "id" obj |> get_int
-                  , get_field "body" obj |> get_string )
-              else None )
-    | Error _ -> None in
+    if not do_edit_existing then None
+    else
+      (* If do edit existing, look for merboco's previous comment. *)
+      match mr_result with
+      | Ok mr -> (
+        match mr.comments with
+        | Some comments -> Comment.of_user ~comments ~user_id
+        | None -> None )
+      | Error _ -> None in
   let note_id result =
     let open Jq in
     try get_dict result |> get_field "id" |> get_int
@@ -56,14 +44,14 @@ let post_or_edit_mr_comment ?(do_edit_existing = true) ?(token : string option)
             note_id:%d"
            id new_id ) ;
       Ok (`Added add_result)
-  | Some (i, prev_body), Ok mr -> (
-      let body = body_of_prev (Some prev_body) in
-      let report_res = report_json i in
+  | Some comment, Ok mr -> (
+      let body = body_of_prev (Some comment.body) in
+      let report_res = report_json comment.id in
       let edit_result =
         if reporting_only then report_res
         else
           Comment.edit_merge_request_note ?token ~project:mr.project.id
-            ~mr:mr.id ~retry_options ~note:i body in
+            ~mr:mr.id ~retry_options ~note:comment.id body in
       match edit_result with
       | `Float 500. ->
           let address =
@@ -84,51 +72,43 @@ let post_or_edit_mr_comment ?(do_edit_existing = true) ?(token : string option)
           Ok (`Added address)
       | _ ->
           let new_id = note_id edit_result in
-          if Int.equal i new_id then
+          if Int.equal comment.id new_id then
             Reporting.add_conditional mr.reports
-              (Fmt.str "A previous comment was edited. mr:%d note-id:%d" mr.id i)
+              (Fmt.str "A previous comment was edited. mr:%d note-id:%d" mr.id
+                 comment.id )
           else
-            Warnings.add ~warnings ~mr
-              ~s:
-                (Fmt.str
-                   "Attempted to edit a comment and might have posted a new \
-                    one. There is a new note id. mr:%d note_id:%d new_id:%d"
-                   mr.id i new_id )
-              () ;
+            Reporting.add_conditional mr.reports
+              (Fmt.str
+                 "Attempted to edit a comment and might have posted a new one. \
+                  There is a new note id. mr:%d note_id:%d new_id:%d"
+                 mr.id comment.id new_id ) ;
+          Warnings.add ~warnings ~mr
+            ~s:
+              (Fmt.str
+                 "Attempted to edit a comment and might have posted a new one. \
+                  There is a new note id. mr:%d note_id:%d new_id:%d"
+                 mr.id comment.id new_id )
+            () ;
           Ok (`Edited edit_result) )
   | _, Error e -> Error e
 
-(** [post_or_edit_mr_thread] will post a Gitlab discussion (thread)
-    [body_of_prev] on merge request [mr_result]. If [do_edit_existing] is true,
-    Merbocop will attempt to edit preexisting thread note before posting a new
-    one. *)
+(** [post_or_edit_mr_thread] will post a Gitlab discussion (thread) [body] on
+    merge request [mr_result]. If [do_edit_existing] is true, Merbocop will
+    attempt to edit preexisting thread note before posting a new one. *)
 let post_or_edit_mr_thread ?(do_edit_existing = true) ?(token : string option)
     ~(mr_result : Merge_request.mr_result) ~(user_id : int)
     ~(retry_options : Web_api.Retry_options.t) ~(warnings : Warnings.t)
     ~(reporting_only : bool) body_of_prev =
   let open Merge_request in
-  let match_comment (c : Comment.t) =
-    match c with
-    | {system= true; _} | {resolvable= false; _} -> None
-    | comment ->
-        if comment.author.id = user_id then Some (comment.id, comment.body)
-        else None in
-  let my_thread : (string * (int * string)) option =
-    (*  TODO  need to get the thread id in there some how*)
-    match mr_result with
-    | Ok _ when not do_edit_existing -> None
-    | Ok mr ->
-        List.find_map
-          (Option.value_exn
-             ~message:
-               "Please provide the --access-token option to edit discussion \
-                threads."
-             mr.threads ) ~f:(fun thread ->
-            let mine = List.find_map thread.comments ~f:match_comment in
-            match mine with
-            | Some comment -> Some (thread.id, comment)
-            | None -> None )
-    | Error _ -> None in
+  let my_thread : (Thread.t * Comment.t) option =
+    if not do_edit_existing then None
+    else
+      match mr_result with
+      | Ok mr -> (
+        match mr.threads with
+        | Some threads -> Thread.of_user ~threads ~user_id
+        | None -> None )
+      | Error _ -> None in
   let thread_id result =
     let open Jq in
     try get_dict result |> get_field "id" |> get_string
@@ -138,18 +118,8 @@ let post_or_edit_mr_thread ?(do_edit_existing = true) ?(token : string option)
         (Exn.to_string_mach e)
         (Ezjsonm.value_to_string result) ;
       raise e in
-  let note_id result =
-    let open Jq in
-    try get_dict result |> get_field "id" |> get_int
-    with e ->
-      Debug.dbg
-        "Bot_actions.post_or_edit_mr_thread note_id error \n\n\n%s, %s\n\n"
-        (Exn.to_string_mach e)
-        (Ezjsonm.value_to_string result) ;
-      raise e in
   let report_thread id =
     Jq.(dict [("id", string id); ("reporting", `String "OK")]) in
-  let report_note id = Jq.(dict [("id", int id); ("reporting", `String "OK")]) in
   match (my_thread, mr_result) with
   | None, Ok mr ->
       let add_result =
@@ -165,14 +135,15 @@ let post_or_edit_mr_thread ?(do_edit_existing = true) ?(token : string option)
             thread-id:%s"
            mr.id new_id ) ;
       Ok (`Added add_result)
-  | Some (id, (note, prev_body)), Ok mr -> (
-      let body = body_of_prev (Some prev_body) in
+  | Some (thread, comment), Ok mr -> (
+      let body = body_of_prev (Some comment.body) in
       let report_res = report_thread "dry-run-no-thread-id" in
       let edit_result =
         if reporting_only then report_res
         else
           Thread.edit_merge_request_discussion ?token ~project:mr.project.id
-            ~mr:mr.id ~retry_options ~discussion:id ~note body in
+            ~mr:mr.id ~retry_options ~discussion:thread.id ~note:comment.id body
+      in
       match edit_result with
       | `Float 500. ->
           let add_ress =
@@ -195,25 +166,44 @@ let post_or_edit_mr_thread ?(do_edit_existing = true) ?(token : string option)
             () ;
           Ok (`Added add_ress)
       | _ ->
+          let note_id result =
+            let open Jq in
+            try get_dict result |> get_field "id" |> get_int
+            with e ->
+              Debug.dbg
+                "Bot_actions.post_or_edit_mr_thread note_id error \n\n\n\
+                 %s, %s\n\n"
+                (Exn.to_string_mach e)
+                (Ezjsonm.value_to_string result) ;
+              raise e in
+          let report_note id =
+            Jq.(dict [("id", int id); ("reporting", `String "OK")]) in
           let new_id =
-            note_id (if reporting_only then report_note note else edit_result)
-            (* API for Editing a descussion note returns only the note JSON info. *)
+            note_id
+              (if reporting_only then report_note comment.id else edit_result)
+            (* API for editing a descussion note returns only the note JSON. So there is not thread id*)
           in
-          if Int.equal note new_id then
+          if Int.equal comment.id new_id then
             Reporting.add_conditional mr.reports
               (Fmt.str
                  "A previous thread note was found and was edited. mr:%d \
                   thread-id:%s note-id:%d"
-                 mr.id id note )
+                 mr.id thread.id comment.id )
           else
-            Warnings.add ~warnings ~mr
-              ~s:
-                (Fmt.str
-                   "Attempted to edit a thread and might have posted a new \
-                    one. There is a new thread id. mr:%d previous thread-id:%s \
-                    new note-id:%d"
-                   mr.id id new_id )
-              () ;
+            Reporting.add_conditional mr.reports
+              (Fmt.str
+                 "Attempted to edit a thread and might have posted a new one. \
+                  There is a new thread id. mr:%d previous thread-id:%s new \
+                  note-id:%d"
+                 mr.id thread.id new_id ) ;
+          Warnings.add ~warnings ~mr
+            ~s:
+              (Fmt.str
+                 "Attempted to edit a thread and might have posted a new one. \
+                  There is a new thread id. mr:%d previous thread-id:%s new \
+                  note-id:%d"
+                 mr.id thread.id new_id )
+            () ;
           Ok (`Edited edit_result) )
   | _, Error e -> Error e
 
diff --git a/src/lib/comment.ml b/src/lib/comment.ml
index 98a579c..53eb900 100644
--- a/src/lib/comment.ml
+++ b/src/lib/comment.ml
@@ -57,7 +57,16 @@ let of_json json : t list =
     Debug.dbg "Comment.of_json %s\n" (Jq.value_to_string json) ;
     raise e
 
-(** JSON Mockups for unitests tests. *)
+(** [of_user c u] is Some comment where user_id matches comment.author from a
+    comment list or None *)
+let of_user ~(comments : t list) ~(user_id : int) : t option =
+  List.find_map comments ~f:(fun c ->
+      match c with
+      | {system= true; _} | {resolvable= true; _} ->
+          (* Ignore system notes or duscussions threads. *) None
+      | {author= pm; _} -> if Int.equal pm.id user_id then Some c else None )
+
+(** JSON Mockups for unit tests. *)
 module Mockup = struct
   let note : int -> string -> Jq.t -> bool -> bool -> Jq.t =
     let open Jq in
diff --git a/src/lib/thread.ml b/src/lib/thread.ml
index 40074b6..ea49663 100644
--- a/src/lib/thread.ml
+++ b/src/lib/thread.ml
@@ -34,6 +34,22 @@ let edit_merge_request_discussion ?token ~project ~mr ~retry_options ~discussion
        (Fmt.str "projects/%d/merge_requests/%d/discussions/%s/notes/%d" project
           mr discussion note ) )
 
+(** [of_user c u] is Some (thread, comment) where user_id matches the author a
+    comment in a partcular thread from a thread list or None *)
+let of_user ~(threads : t list) ~(user_id : int) : (t * Comment.t) option =
+  let match_comment comments =
+    let open Comment in
+    List.find_map comments ~f:(fun c ->
+        match c with
+        | {system= true; _} | {resolvable= false; _} ->
+            (* Ignore system notes or duscussions threads. *) None
+        | comment ->
+            if Int.equal comment.author.id user_id then Some c else None ) in
+  List.find_map threads ~f:(fun th ->
+      match match_comment th.comments with
+      | Some c -> Some (th, c)
+      | None -> None )
+
 (** A list of merge request Threads. **)
 let of_json json : t list =
   let threads =
-- 
GitLab


From 1ef5729a355353758f19f5a6db2596048f23bc10 Mon Sep 17 00:00:00 2001
From: Phil Saxton <phillip.saxton@tqtezos.com>
Date: Fri, 20 Jan 2023 12:11:23 -0500
Subject: [PATCH 3/8] comment/comment_body: Improve `Comment.is_new`.

Make is easier to compare new and previous comments by separating and
ignoring the comment footer. This splits module Md from Comment_body
to prevent circular dependencies.

	new file:   src/lib/Md.ml
	modified:   src/lib/comment.ml
	modified:   src/lib/comment_body.ml
---
 src/lib/Md.ml           | 49 +++++++++++++++++++++++++++++++++++++++++
 src/lib/comment.ml      | 15 +++++++++++--
 src/lib/comment_body.ml | 43 +-----------------------------------
 3 files changed, 63 insertions(+), 44 deletions(-)
 create mode 100644 src/lib/Md.ml

diff --git a/src/lib/Md.ml b/src/lib/Md.ml
new file mode 100644
index 0000000..9545eef
--- /dev/null
+++ b/src/lib/Md.ml
@@ -0,0 +1,49 @@
+open! Base
+open Fmt
+
+let empty = ""
+let h n s = [str "%s %s" (String.make n '#') s; empty]
+let par l = l @ [empty]
+let verbatim l = "```" :: l @ ["```"]
+let plural = function 1 -> "" | _ -> "s"
+let plural_list = function [_] -> "" | _ -> "s"
+let plural_be = function 1 -> "is" | _ -> "are"
+let third_person = function 1 -> "s" | _ -> ""
+let hl = [""; String.make 10 '-'; ""]
+let colon_or_dot = function 0 -> "." | _ -> ":"
+
+let merge_team_doc_link =
+  "http://tezos.gitlab.io/developer/contributing.html#the-merge-request-bot"
+
+let todo_comment_doc_link =
+  "https://tezos.gitlab.io/developer/guidelines.html#todo-fixme-comments"
+
+let code_escape s =
+  let max_consecutive_backquotes =
+    String.fold s ~init:(0, 0) ~f:(fun (prev_c, prev_m) -> function
+      | '`' -> (prev_c + 1, prev_m) | _ -> (0, max prev_m prev_c) )
+    |> snd in
+  let escaper = String.make (max_consecutive_backquotes + 1) '`' in
+  String.concat ~sep:""
+    [ escaper; (match s.[0] with '`' -> " " | _ -> "" | exception _ -> " "); s
+    ; ( match s.[String.length s - 1] with
+      | '`' -> " "
+      | _ -> ""
+      | exception _ -> " " ); escaper ]
+
+let one_couple_few nb =
+  match nb with
+  | 0 -> "no"
+  | 1 -> "one"
+  | 2 | 3 | 4 -> "a couple of"
+  | _ -> "a few"
+
+(** [horizontal n] is a string representing a markdown horizontal line. n must
+    be greater than 2. Merbocop uses this for parsing its own comments. Please
+    don't confuse Merbocop by using the same number [n] in multiple places. See
+    Delimiter module below. *)
+let horizontal int =
+  let rec aux i s = match i with 0 -> "" ^ s | i -> aux (i - 1) ("-" ^ s) in
+  aux int ""
+
+module Delimiters = struct let comment_footer = horizontal 6 end
diff --git a/src/lib/comment.ml b/src/lib/comment.ml
index 53eb900..7128b92 100644
--- a/src/lib/comment.ml
+++ b/src/lib/comment.ml
@@ -57,8 +57,19 @@ let of_json json : t list =
     Debug.dbg "Comment.of_json %s\n" (Jq.value_to_string json) ;
     raise e
 
-(** [of_user c u] is Some comment where user_id matches comment.author from a
-    comment list or None *)
+(** [is_new b c] is true if new comment [b] is not equal to a
+    merge_request.comment.body [c.body] and false otherwise. *)
+let is_new ~(body : string) ~(comment : t) : bool =
+  let b =
+    (* The comment footer include the current date so it is ignored. *)
+    match String.substr_index body ~pattern:Md.Delimiters.comment_footer with
+    | None -> body
+    | Some i -> String.prefix body i in
+  let c = comment.body in
+  not (String.is_prefix c ~prefix:b)
+
+(** [of_user c u] is Some comment where [u] matches comment.author from a list
+    [c] or None *)
 let of_user ~(comments : t list) ~(user_id : int) : t option =
   List.find_map comments ~f:(fun c ->
       match c with
diff --git a/src/lib/comment_body.ml b/src/lib/comment_body.ml
index 9212f6a..4026258 100644
--- a/src/lib/comment_body.ml
+++ b/src/lib/comment_body.ml
@@ -1,46 +1,5 @@
 open! Base
 
-module Md = struct
-  open Fmt
-
-  let empty = ""
-  let h n s = [str "%s %s" (String.make n '#') s; empty]
-  let par l = l @ [empty]
-  let verbatim l = "```" :: l @ ["```"]
-  let plural = function 1 -> "" | _ -> "s"
-  let plural_list = function [_] -> "" | _ -> "s"
-  let plural_be = function 1 -> "is" | _ -> "are"
-  let third_person = function 1 -> "s" | _ -> ""
-  let hl = [""; String.make 10 '-'; ""]
-  let colon_or_dot = function 0 -> "." | _ -> ":"
-
-  let merge_team_doc_link =
-    "http://tezos.gitlab.io/developer/contributing.html#the-merge-request-bot"
-
-  let todo_comment_doc_link =
-    "https://tezos.gitlab.io/developer/guidelines.html#todo-fixme-comments"
-
-  let code_escape s =
-    let max_consecutive_backquotes =
-      String.fold s ~init:(0, 0) ~f:(fun (prev_c, prev_m) -> function
-        | '`' -> (prev_c + 1, prev_m) | _ -> (0, max prev_m prev_c) )
-      |> snd in
-    let escaper = String.make (max_consecutive_backquotes + 1) '`' in
-    String.concat ~sep:""
-      [ escaper; (match s.[0] with '`' -> " " | _ -> "" | exception _ -> " "); s
-      ; ( match s.[String.length s - 1] with
-        | '`' -> " "
-        | _ -> ""
-        | exception _ -> " " ); escaper ]
-
-  let one_couple_few nb =
-    match nb with
-    | 0 -> "no"
-    | 1 -> "one"
-    | 2 | 3 | 4 -> "a couple of"
-    | _ -> "a few"
-end
-
 let pipeline_links () =
   match
     (Caml.Sys.getenv_opt "CI_PIPELINE_ID", Caml.Sys.getenv_opt "CI_JOB_ID")
@@ -60,7 +19,7 @@ let simple_message ?(with_footer = true) ?with_touched_files_tags mr =
     if not with_footer then []
     else
       par
-        [ str "------"; ""
+        [ str "%s" Delimiters.comment_footer; ""
         ; str "I, 🤖 👮, ran this inspection on `%s`."
             ( System.command_to_string_list "date -R"
             |> fst |> String.concat ~sep:"" )
-- 
GitLab


From 6a5985e6554effaf3c163cb5b09318281ad90b3f Mon Sep 17 00:00:00 2001
From: Phil Saxton <phillip.saxton@tqtezos.com>
Date: Fri, 20 Jan 2023 15:10:59 -0500
Subject: [PATCH 4/8] bot_actions: Check prev Comments and Threads

Before posting new comments or threads, Merbocop will check that the
new comment is different from the previous before editing.
---
 src/app/merbocop.ml    | 13 ++++++++----
 src/lib/bot_actions.ml | 47 ++++++++++++++++++++++++++++--------------
 2 files changed, 40 insertions(+), 20 deletions(-)

diff --git a/src/app/merbocop.ml b/src/app/merbocop.ml
index cb2b924..2e7b375 100644
--- a/src/app/merbocop.ml
+++ b/src/app/merbocop.ml
@@ -264,7 +264,7 @@ let post_comments (self_id : int option) (token : string option)
     List.iter mr_list ~f:(function
       | _, Error (`Mr_not_found _) -> (* Already counted *) ()
       | project, Ok mr -> (
-          let body_of_prev _previous_body =
+          let body =
             try Comment_body.simple_message mr |> String.concat ~sep:"\n"
             with e ->
               Fmt.str "ERROR: %a (%d MRs)" Exn.pp e (List.length mr_list) in
@@ -280,7 +280,7 @@ let post_comments (self_id : int option) (token : string option)
               ~user_id:
                 (Option.value_exn
                    ~message:"Commenting requires the --self-id option." self_id )
-              ~retry_options ~warnings ~reporting_only body_of_prev
+              ~retry_options ~warnings ~reporting_only body
           with
           | Ok (`Added res) | Ok (`Edited res) -> (
               say "Ok, done %s/%a %s." project Merge_request.pp_quick (Ok mr)
@@ -294,6 +294,11 @@ let post_comments (self_id : int option) (token : string option)
                     [ Fmt.str "Add-or-edit-comment: %s/%a: %s" project
                         Merge_request.pp_quick (Ok mr) msg ]
                 | _ -> [] )
+          | Ok (`No_change json) ->
+              say "Nothing new to say: %s/%a %s" project Merge_request.pp_quick
+                (Ok mr)
+                ( Jq.value_to_string ~minify:true json
+                |> fun s -> String.prefix s 35 )
           | Error (`Mr_not_found (proj, s, js, e)) ->
               say "Cannot comment, no MR there: %s/%s\n%s\n%a" proj s
                 (Ezjsonm.value_to_string ~minify:false js)
@@ -322,7 +327,7 @@ let no_comment_editing () =
     [mr_list] to [write_body]. *)
 let full_report_body (mr_list : (string * Merge_request.mr_result) list)
     (errors : string list ref) (write_body : string option) =
-  let body_of_prev _previous_body =
+  let body =
     try Comment_body.full_report () ~mr_list:(List.map ~f:snd mr_list)
     with e ->
       errors := !errors @ [Fmt.str "Making multi-mr body: %a" Exn.pp e] ;
@@ -332,7 +337,7 @@ let full_report_body (mr_list : (string * Merge_request.mr_result) list)
     | Some p -> p
     | None -> Caml.Filename.temp_file "body" ".md" in
   let o = Caml.open_out tmp_md in
-  Caml.output_string o (body_of_prev None) ;
+  Caml.output_string o body ;
   Caml.close_out o ;
   say "Body: file://%s\n" tmp_md
 
diff --git a/src/lib/bot_actions.ml b/src/lib/bot_actions.ml
index 4f9f5c8..b59b69c 100644
--- a/src/lib/bot_actions.ml
+++ b/src/lib/bot_actions.ml
@@ -7,7 +7,7 @@ open! Base
 let post_or_edit_mr_comment ?(do_edit_existing = true) ?(token : string option)
     ~(mr_result : Merge_request.mr_result) ~(user_id : int)
     ~(retry_options : Web_api.Retry_options.t) ~(warnings : Warnings.t)
-    ~(reporting_only : bool) body_of_prev =
+    ~(reporting_only : bool) body =
   let open Merge_request in
   let my_comment =
     if not do_edit_existing then None
@@ -34,7 +34,6 @@ let post_or_edit_mr_comment ?(do_edit_existing = true) ?(token : string option)
       let add_result =
         if reporting_only then report_json 000000
         else
-          let body = body_of_prev None in
           Comment.add_merge_request_note ?token ~project:project.id ~mr:id
             ~retry_options body in
       let new_id = note_id add_result in
@@ -45,14 +44,23 @@ let post_or_edit_mr_comment ?(do_edit_existing = true) ?(token : string option)
            id new_id ) ;
       Ok (`Added add_result)
   | Some comment, Ok mr -> (
-      let body = body_of_prev (Some comment.body) in
-      let report_res = report_json comment.id in
-      let edit_result =
-        if reporting_only then report_res
-        else
-          Comment.edit_merge_request_note ?token ~project:mr.project.id
-            ~mr:mr.id ~retry_options ~note:comment.id body in
-      match edit_result with
+    (* Check if comment body is a new comment *)
+    match Comment.is_new ~body ~comment with
+    | false ->
+        Reporting.add_conditional mr.reports
+          (Fmt.str
+             "A previous comment was found but wasn't edited because there was \
+              nothing new to say mr:%d note_id:%d"
+             mr.id comment.id ) ;
+        Ok (`No_change comment.json)
+    | true -> (
+        let report_res = report_json comment.id in
+        let edit_result =
+          if reporting_only then report_res
+          else
+            Comment.edit_merge_request_note ?token ~project:mr.project.id
+              ~mr:mr.id ~retry_options ~note:comment.id body in
+        match edit_result with
       | `Float 500. ->
           let address =
             if reporting_only then report_res
@@ -98,7 +106,7 @@ let post_or_edit_mr_comment ?(do_edit_existing = true) ?(token : string option)
 let post_or_edit_mr_thread ?(do_edit_existing = true) ?(token : string option)
     ~(mr_result : Merge_request.mr_result) ~(user_id : int)
     ~(retry_options : Web_api.Retry_options.t) ~(warnings : Warnings.t)
-    ~(reporting_only : bool) body_of_prev =
+    ~(reporting_only : bool) body =
   let open Merge_request in
   let my_thread : (Thread.t * Comment.t) option =
     if not do_edit_existing then None
@@ -125,7 +133,6 @@ let post_or_edit_mr_thread ?(do_edit_existing = true) ?(token : string option)
       let add_result =
         if reporting_only then report_thread "dry-run-no-thread-id"
         else
-          let body = body_of_prev None in
           Thread.start_merge_request_discussion ?token ~project:mr.project.id
             ~mr:mr.id ~retry_options body in
       let new_id = thread_id add_result in
@@ -136,10 +143,18 @@ let post_or_edit_mr_thread ?(do_edit_existing = true) ?(token : string option)
            mr.id new_id ) ;
       Ok (`Added add_result)
   | Some (thread, comment), Ok mr -> (
-      let body = body_of_prev (Some comment.body) in
-      let report_res = report_thread "dry-run-no-thread-id" in
-      let edit_result =
-        if reporting_only then report_res
+    match Comment.is_new ~body ~comment with
+    | false ->
+        Reporting.add_conditional mr.reports
+          (Fmt.str
+             "A previous thread was found but wasn't edited because there was \
+              nothing new to say mr:%d thread_id:%s note_id:%d"
+             mr.id thread.id comment.id ) ;
+        Ok (`No_change comment.json)
+    | true -> (
+        let report_res = report_thread "dry-run-no-thread-id" in
+        let edit_result =
+          if reporting_only then report_res
         else
           Thread.edit_merge_request_discussion ?token ~project:mr.project.id
             ~mr:mr.id ~retry_options ~discussion:thread.id ~note:comment.id body
-- 
GitLab


From befb155ef1d35c6cdf56a3c5e07d125e51cee355 Mon Sep 17 00:00:00 2001
From: Phil Saxton <phillip.saxton@tqtezos.com>
Date: Fri, 20 Jan 2023 15:40:44 -0500
Subject: [PATCH 5/8] bot_action: Fix-up formating and variable names.

---
 src/lib/bot_actions.ml | 204 ++++++++++++++++++++---------------------
 1 file changed, 102 insertions(+), 102 deletions(-)

diff --git a/src/lib/bot_actions.ml b/src/lib/bot_actions.ml
index b59b69c..5d2cf93 100644
--- a/src/lib/bot_actions.ml
+++ b/src/lib/bot_actions.ml
@@ -54,50 +54,50 @@ let post_or_edit_mr_comment ?(do_edit_existing = true) ?(token : string option)
              mr.id comment.id ) ;
         Ok (`No_change comment.json)
     | true -> (
-        let report_res = report_json comment.id in
+        let report_result = report_json comment.id in
         let edit_result =
-          if reporting_only then report_res
+          if reporting_only then report_result
           else
             Comment.edit_merge_request_note ?token ~project:mr.project.id
               ~mr:mr.id ~retry_options ~note:comment.id body in
         match edit_result with
-      | `Float 500. ->
-          let address =
-            if reporting_only then report_res
-            else
-              Comment.add_merge_request_note ?token ~project:mr.project.id
-                ~mr:mr.id ~retry_options body in
-          let new_id = note_id address in
-          Reporting.add_conditional mr.reports
-            (Fmt.str "Failed to edit comment with response code 500.") ;
-          Warnings.add ~warnings ~mr
-            ~s:
-              (Fmt.str
-                 "Failed to edit comment with response code 500. A new comment \
-                  was posted mr:%d note_id:%d"
-                 mr.id new_id )
-            () ;
-          Ok (`Added address)
-      | _ ->
-          let new_id = note_id edit_result in
-          if Int.equal comment.id new_id then
+        | `Float 500. ->
+            let add_result =
+              if reporting_only then report_result
+              else
+                Comment.add_merge_request_note ?token ~project:mr.project.id
+                  ~mr:mr.id ~retry_options body in
+            let new_id = note_id add_result in
             Reporting.add_conditional mr.reports
-              (Fmt.str "A previous comment was edited. mr:%d note-id:%d" mr.id
-                 comment.id )
-          else
-            Reporting.add_conditional mr.reports
-              (Fmt.str
-                 "Attempted to edit a comment and might have posted a new one. \
-                  There is a new note id. mr:%d note_id:%d new_id:%d"
-                 mr.id comment.id new_id ) ;
-          Warnings.add ~warnings ~mr
-            ~s:
-              (Fmt.str
-                 "Attempted to edit a comment and might have posted a new one. \
-                  There is a new note id. mr:%d note_id:%d new_id:%d"
-                 mr.id comment.id new_id )
-            () ;
-          Ok (`Edited edit_result) )
+              (Fmt.str "Failed to edit comment with response code 500.") ;
+            Warnings.add ~warnings ~mr
+              ~s:
+                (Fmt.str
+                   "Failed to edit comment with response code 500. A new \
+                    comment was posted mr:%d note_id:%d"
+                   mr.id new_id )
+              () ;
+            Ok (`Added add_result)
+        | _ ->
+            let new_id = note_id edit_result in
+            if Int.equal comment.id new_id then
+              Reporting.add_conditional mr.reports
+                (Fmt.str "A previous comment was edited. mr:%d note-id:%d" mr.id
+                   comment.id )
+            else
+              Reporting.add_conditional mr.reports
+                (Fmt.str
+                   "Attempted to edit a comment and might have posted a new \
+                    one. There is a new note id. mr:%d note_id:%d new_id:%d"
+                   mr.id comment.id new_id ) ;
+            Warnings.add ~warnings ~mr
+              ~s:
+                (Fmt.str
+                   "Attempted to edit a comment and might have posted a new \
+                    one. There is a new note id. mr:%d note_id:%d new_id:%d"
+                   mr.id comment.id new_id )
+              () ;
+            Ok (`Edited edit_result) ) )
   | _, Error e -> Error e
 
 (** [post_or_edit_mr_thread] will post a Gitlab discussion (thread) [body] on
@@ -152,74 +152,74 @@ let post_or_edit_mr_thread ?(do_edit_existing = true) ?(token : string option)
              mr.id thread.id comment.id ) ;
         Ok (`No_change comment.json)
     | true -> (
-        let report_res = report_thread "dry-run-no-thread-id" in
+        let report_result = report_thread "dry-run-no-thread-id" in
         let edit_result =
-          if reporting_only then report_res
-        else
-          Thread.edit_merge_request_discussion ?token ~project:mr.project.id
-            ~mr:mr.id ~retry_options ~discussion:thread.id ~note:comment.id body
-      in
-      match edit_result with
-      | `Float 500. ->
-          let add_ress =
-            if reporting_only then report_res
-            else
-              Thread.start_merge_request_discussion ?token
-                ~project:mr.project.id ~mr:mr.id ~retry_options body in
-          let new_id = thread_id add_ress in
-          Reporting.add_conditional mr.reports
-            (Fmt.str
-               "Failed to edit a previous threat with response code 500. A new \
-                thread was posted mr:%d thread-id:%s"
-               mr.id new_id ) ;
-          Warnings.add ~warnings ~mr
-            ~s:
-              (Fmt.str
-                 "Failed to edit a previous threat with response code 500. A \
-                  new thread was posted mr:%d thread-id:%s"
-                 mr.id new_id )
-            () ;
-          Ok (`Added add_ress)
-      | _ ->
-          let note_id result =
-            let open Jq in
-            try get_dict result |> get_field "id" |> get_int
-            with e ->
-              Debug.dbg
-                "Bot_actions.post_or_edit_mr_thread note_id error \n\n\n\
-                 %s, %s\n\n"
-                (Exn.to_string_mach e)
-                (Ezjsonm.value_to_string result) ;
-              raise e in
-          let report_note id =
-            Jq.(dict [("id", int id); ("reporting", `String "OK")]) in
-          let new_id =
-            note_id
-              (if reporting_only then report_note comment.id else edit_result)
-            (* API for editing a descussion note returns only the note JSON. So there is not thread id*)
-          in
-          if Int.equal comment.id new_id then
-            Reporting.add_conditional mr.reports
-              (Fmt.str
-                 "A previous thread note was found and was edited. mr:%d \
-                  thread-id:%s note-id:%d"
-                 mr.id thread.id comment.id )
+          if reporting_only then report_result
           else
+            Thread.edit_merge_request_discussion ?token ~project:mr.project.id
+              ~mr:mr.id ~retry_options ~discussion:thread.id ~note:comment.id
+              body in
+        match edit_result with
+        | `Float 500. ->
+            let add_result =
+              if reporting_only then report_result
+              else
+                Thread.start_merge_request_discussion ?token
+                  ~project:mr.project.id ~mr:mr.id ~retry_options body in
+            let new_id = thread_id add_result in
             Reporting.add_conditional mr.reports
               (Fmt.str
-                 "Attempted to edit a thread and might have posted a new one. \
-                  There is a new thread id. mr:%d previous thread-id:%s new \
-                  note-id:%d"
-                 mr.id thread.id new_id ) ;
-          Warnings.add ~warnings ~mr
-            ~s:
-              (Fmt.str
-                 "Attempted to edit a thread and might have posted a new one. \
-                  There is a new thread id. mr:%d previous thread-id:%s new \
-                  note-id:%d"
-                 mr.id thread.id new_id )
-            () ;
-          Ok (`Edited edit_result) )
+                 "Failed to edit a previous threat with response code 500. A \
+                  new thread was posted mr:%d thread-id:%s"
+                 mr.id new_id ) ;
+            Warnings.add ~warnings ~mr
+              ~s:
+                (Fmt.str
+                   "Failed to edit a previous threat with response code 500. A \
+                    new thread was posted mr:%d thread-id:%s"
+                   mr.id new_id )
+              () ;
+            Ok (`Added add_result)
+        | _ ->
+            let note_id result =
+              let open Jq in
+              try get_dict result |> get_field "id" |> get_int
+              with e ->
+                Debug.dbg
+                  "Bot_actions.post_or_edit_mr_thread note_id error \n\n\n\
+                   %s, %s\n\n"
+                  (Exn.to_string_mach e)
+                  (Ezjsonm.value_to_string result) ;
+                raise e in
+            let report_note id =
+              Jq.(dict [("id", int id); ("reporting", `String "OK")]) in
+            let new_id =
+              note_id
+                (if reporting_only then report_note comment.id else edit_result)
+              (* API for editing a descussion note returns only the note JSON. So there is not thread id*)
+            in
+            if Int.equal comment.id new_id then
+              Reporting.add_conditional mr.reports
+                (Fmt.str
+                   "A previous thread note was found and was edited. mr:%d \
+                    thread-id:%s note-id:%d"
+                   mr.id thread.id comment.id )
+            else
+              Reporting.add_conditional mr.reports
+                (Fmt.str
+                   "Attempted to edit a thread and might have posted a new \
+                    one. There is a new thread id. mr:%d previous thread-id:%s \
+                    new note-id:%d"
+                   mr.id thread.id new_id ) ;
+            Warnings.add ~warnings ~mr
+              ~s:
+                (Fmt.str
+                   "Attempted to edit a thread and might have posted a new \
+                    one. There is a new thread id. mr:%d previous thread-id:%s \
+                    new note-id:%d"
+                   mr.id thread.id new_id )
+              () ;
+            Ok (`Edited edit_result) ) )
   | _, Error e -> Error e
 
 (** [post_or_edit_mr_comment_or_thread] will post a merge request thread if
-- 
GitLab


From ff86a0634d0c3de8308af207054e53226f957303 Mon Sep 17 00:00:00 2001
From: Phil Saxton <phillip.saxton@tqtezos.com>
Date: Fri, 20 Jan 2023 16:01:05 -0500
Subject: [PATCH 6/8] main: Better cli print for post_comment `No_change

---
 src/app/merbocop.ml | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/app/merbocop.ml b/src/app/merbocop.ml
index 2e7b375..c763c8b 100644
--- a/src/app/merbocop.ml
+++ b/src/app/merbocop.ml
@@ -295,10 +295,10 @@ let post_comments (self_id : int option) (token : string option)
                         Merge_request.pp_quick (Ok mr) msg ]
                 | _ -> [] )
           | Ok (`No_change json) ->
-              say "Nothing new to say: %s/%a %s" project Merge_request.pp_quick
-                (Ok mr)
+              say "Nothing new to say: %s/%a %s...}" project
+                Merge_request.pp_quick (Ok mr)
                 ( Jq.value_to_string ~minify:true json
-                |> fun s -> String.prefix s 35 )
+                |> fun s -> String.prefix s 41 )
           | Error (`Mr_not_found (proj, s, js, e)) ->
               say "Cannot comment, no MR there: %s/%s\n%s\n%a" proj s
                 (Ezjsonm.value_to_string ~minify:false js)
-- 
GitLab


From dfe51fe9933a44c47c3a23e70470788ccda3d8a5 Mon Sep 17 00:00:00 2001
From: Phil Saxton <phillip.saxton@tqtezos.com>
Date: Thu, 26 Jan 2023 16:38:43 -0500
Subject: [PATCH 7/8] bot_actions/reviewers: Propagate --dry-run option.

This connects the --dry-run option to the reporting_only toggle on
Merbocop's api calls. This will stop the PUT/POST requests while still
producing the full-report.

	modified:   src/app/merbocop.ml
	modified:   src/lib/bot_actions.ml
	modified:   src/lib/reviewers.ml
---
 src/app/merbocop.ml    | 21 ++++++++++++---------
 src/lib/bot_actions.ml | 20 ++++++++++++++------
 src/lib/reviewers.ml   |  5 +++--
 3 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/src/app/merbocop.ml b/src/app/merbocop.ml
index c763c8b..3d5aee0 100644
--- a/src/app/merbocop.ml
+++ b/src/app/merbocop.ml
@@ -119,7 +119,7 @@ let limit_to_recent_updates_term () =
 (** [label_mr] applies (or removes) a Gitlab, label of [doc_label] and/or
     [approved_label] for all merger request in [mr_list]. *)
 let label_mr (self_id : int option) (token : string option)
-    (retry_options : Web_api.Retry_options.t)
+    (retry_options : Web_api.Retry_options.t) (reporting_only : bool)
     (mr_list : (string * Merge_request.mr_result) list)
     (errors : string list ref) (approving : bool) (doc_label : string option)
     (approved_label : string option) =
@@ -157,9 +157,9 @@ let label_mr (self_id : int option) (token : string option)
                 | _ -> (add, remove @ to_list approved_label) ) ) in
         match
           ( Bot_actions.label_mr ?token ~project ~mr ~retry_options
-              (fst add_remove_lists)
+              ~reporting_only (fst add_remove_lists)
           , Bot_actions.unlabel_mr ?token ~project ~mr ~retry_options
-              (snd add_remove_lists) )
+              ~reporting_only (snd add_remove_lists) )
         with
         | Ok (Some (_ : Ezjsonm.value)), Ok (Some (_ : Ezjsonm.value))
          |Ok (Some (_ : Ezjsonm.value)), Ok None
@@ -198,13 +198,15 @@ let set_approved_label () =
 (** [approve_mr] approves (or unapproves) each merger request in [mr_list] based
     on its [`Doc_only | `Not_doc_only] status. *)
 let approve_mr (self_id : int option) (token : string option)
-    (retry_options : Web_api.Retry_options.t)
+    (retry_options : Web_api.Retry_options.t) (reporting_only : bool)
     (mr_list : (string * Merge_request.mr_result) list)
     (errors : string list ref) (approve_doc_only_mr : bool) =
   List.iter mr_list ~f:(function
     | _, Error (`Mr_not_found _) -> ()
     | project, Ok mr ->
-        let reporting_only = if approve_doc_only_mr then false else true in
+        let reporting_only =
+          if approve_doc_only_mr && not reporting_only then false else true
+        in
         List.iter (Analysis.labels_of_mr mr) ~f:(function
           | `Doc_only -> (
             match
@@ -528,10 +530,11 @@ let () =
            if comment_on_self then
              post_comments self token mr_list retry errors comment_on_self
                job_warnings do_edit_existing resolve_required report_only ;
-           label_mr self token retry mr_list errors approve_doc_only doc_label
-             approved_label ;
-           approve_mr self token retry mr_list errors approve_doc_only ;
-           Reviewers.exe token retry mr_list errors assign_reviewers ;
+           label_mr self token retry report_only mr_list errors approve_doc_only
+             doc_label approved_label ;
+           approve_mr self token retry report_only mr_list errors
+             approve_doc_only ;
+           Reviewers.exe token retry report_only mr_list errors assign_reviewers ;
            full_report_body mr_list errors write_body ;
            post_warnings post_warnings_to token retry errors job_warnings ) ;
           match !errors with
diff --git a/src/lib/bot_actions.ml b/src/lib/bot_actions.ml
index 5d2cf93..9f17665 100644
--- a/src/lib/bot_actions.ml
+++ b/src/lib/bot_actions.ml
@@ -254,7 +254,9 @@ let check_json ?(message = "Gitlab API call") (api_call : Ezjsonm.value) : Jq.t
 (** [label_mr token project mr labels] adds Gitlab [labels] to a merge request. *)
 let label_mr ?(token : string option) ~(project : string)
     ~(mr : Merge_request.t) ~(retry_options : Web_api.Retry_options.t)
-    (labels : string list) =
+    ~(reporting_only : bool) (labels : string list) =
+  let report_json =
+    Ezjsonm.(dict [("id", int mr.id); ("reporting", `String "OK")]) in
   match
     List.filter labels ~f:(fun label ->
         List.mem mr.labels label ~equal:String.equal |> not )
@@ -264,8 +266,10 @@ let label_mr ?(token : string option) ~(project : string)
     try
       Ok
         (Some
-           ( Gitlab.add_mr_labels ?token ~project ~mr_id:mr.id ~retry_options
-               labels
+           ( ( if reporting_only then report_json
+             else
+               Gitlab.add_mr_labels ?token ~project ~mr_id:mr.id ~retry_options
+                 labels )
            |> check_json
                 ~message:
                   (Printf.sprintf "Gitlab.add_mr_labels project:%s mr:%d"
@@ -276,7 +280,9 @@ let label_mr ?(token : string option) ~(project : string)
     request. *)
 let unlabel_mr ?(token : string option) ~(project : string)
     ~(mr : Merge_request.t) ~(retry_options : Web_api.Retry_options.t)
-    (labels : string list) =
+    ~(reporting_only : bool) (labels : string list) =
+  let report_json =
+    Ezjsonm.(dict [("id", int mr.id); ("reporting", `String "OK")]) in
   match
     List.filter labels ~f:(fun label ->
         List.mem mr.labels label ~equal:String.equal )
@@ -286,8 +292,10 @@ let unlabel_mr ?(token : string option) ~(project : string)
     try
       Ok
         (Some
-           ( Gitlab.remove_mr_labels ?token ~project ~mr_id:mr.id ~retry_options
-               labels
+           ( ( if reporting_only then report_json
+             else
+               Gitlab.remove_mr_labels ?token ~project ~mr_id:mr.id
+                 ~retry_options labels )
            |> check_json
                 ~message:
                   (Printf.sprintf "Gitlab.remove_mr_labels project:%s mr:%d"
diff --git a/src/lib/reviewers.ml b/src/lib/reviewers.ml
index 684bb3c..ff6d5b8 100644
--- a/src/lib/reviewers.ml
+++ b/src/lib/reviewers.ml
@@ -146,11 +146,12 @@ let assign_or_report ?(token : string option) ~(project : string)
                    mr.id ) ) )
   with e -> Error e
 
-let exe token retry_options mr_list errors assign_reviewers =
+let exe token retry_options reporting_only mr_list errors assign_reviewers =
   List.iter mr_list ~f:(function
     | _, Error (`Mr_not_found _) -> ()
     | project, Ok mr -> (
-        let reporting_only = if assign_reviewers then false else true in
+        let reporting_only =
+          if assign_reviewers && not reporting_only then false else true in
         let reviewers =
           List.concat_map (new_reviewers_and_report ~mr) ~f:(fun rev ->
               Project_members.(of_uname ~retry_options rev |> of_json) )
-- 
GitLab


From d7ba64447a1148cd895f813d6cea83f134320a6c Mon Sep 17 00:00:00 2001
From: Phil Saxton <phillip.saxton@tqtezos.com>
Date: Fri, 27 Jan 2023 14:06:39 -0500
Subject: [PATCH 8/8] main/reviewers: Make booleans more clear.

---
 src/app/merbocop.ml  | 4 +---
 src/lib/reviewers.ml | 3 +--
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/app/merbocop.ml b/src/app/merbocop.ml
index 3d5aee0..2d0ed88 100644
--- a/src/app/merbocop.ml
+++ b/src/app/merbocop.ml
@@ -204,9 +204,7 @@ let approve_mr (self_id : int option) (token : string option)
   List.iter mr_list ~f:(function
     | _, Error (`Mr_not_found _) -> ()
     | project, Ok mr ->
-        let reporting_only =
-          if approve_doc_only_mr && not reporting_only then false else true
-        in
+        let reporting_only = reporting_only || not approve_doc_only_mr in
         List.iter (Analysis.labels_of_mr mr) ~f:(function
           | `Doc_only -> (
             match
diff --git a/src/lib/reviewers.ml b/src/lib/reviewers.ml
index ff6d5b8..e1a6f7f 100644
--- a/src/lib/reviewers.ml
+++ b/src/lib/reviewers.ml
@@ -150,8 +150,7 @@ let exe token retry_options reporting_only mr_list errors assign_reviewers =
   List.iter mr_list ~f:(function
     | _, Error (`Mr_not_found _) -> ()
     | project, Ok mr -> (
-        let reporting_only =
-          if assign_reviewers && not reporting_only then false else true in
+        let reporting_only = reporting_only || not assign_reviewers in
         let reviewers =
           List.concat_map (new_reviewers_and_report ~mr) ~f:(fun rev ->
               Project_members.(of_uname ~retry_options rev |> of_json) )
-- 
GitLab