Merge pull request #46 from mbarbin/naming-and-doc

mbarbin · web-flow · commit 2c2b7ccd75f6 · 2025-07-03T15:46:05.000+02:00
Naming and doc
diff --git a/CHANGES.md b/CHANGES.md
@@ -6,6 +6,7 @@
 
 ### Changed
 
+- A breaking renaming in the terminology (kind => status) (#46, @mbarbin).
 - Add buffer line header in emacs crs-grep-mode by default (#44, @mbarbin).
 
 ### Fixed
diff --git a/lib/cr_comment/src/cr_comment.ml b/lib/cr_comment/src/cr_comment.ml
@@ -19,7 +19,7 @@
 (*  <http://www.gnu.org/licenses/> and <https://spdx.org>, respectively.        *)
 (********************************************************************************)
 
-module Kind = Kind
+module Status = Status
 module Filter = Filter
 module Priority = Priority
 module Qualifier = Qualifier
diff --git a/lib/cr_comment/src/cr_comment.mli b/lib/cr_comment/src/cr_comment.mli
@@ -19,7 +19,7 @@
 (*_  <http://www.gnu.org/licenses/> and <https://spdx.org>, respectively.        *)
 (*_*******************************************************************************)
 
-module Kind = Kind
+module Status = Status
 module Filter = Filter
 module Priority = Priority
 module Qualifier = Qualifier
diff --git a/lib/cr_comment/src/cr_comment0.ml b/lib/cr_comment/src/cr_comment0.ml
@@ -62,7 +62,7 @@
    * - Remove support for extra headers.
    * - Remove support for attributes.
    * - Remove assignee computation (left as external work).
-   * - Replace [is_xcr] by a variant type [Kind.t].
+   * - Replace [is_xcr] by a variant type [Status.t].
    * - Make [reporter] mandatory.
    * - Refactor [Raw], make [t] a record with a processed part that may fail.
    * - Compute [digest_of_condensed_content] for all CR kinds.
@@ -85,7 +85,7 @@ module Header = struct
     [@@@coverage off]
 
     type t =
-      { kind : Kind.t Loc.Txt.t
+      { status : Status.t Loc.Txt.t
       ; qualifier : Qualifier.t Loc.Txt.t
       ; reporter : Vcs.User_handle.t Loc.Txt.t
       ; recipient : Vcs.User_handle.t Loc.Txt.t option
@@ -98,27 +98,30 @@ module Header = struct
   module With_loc = struct
     let reporter t = t.reporter
     let recipient t = t.recipient
-    let kind t = t.kind
+    let status t = t.status
     let qualifier t = t.qualifier
 
     (* Deprecated. *)
     let reported_by = reporter
     let for_ = recipient
     let due = qualifier
+    let kind = status
   end
 
-  let create ~kind ~qualifier ~reporter ~recipient =
-    { kind; qualifier; reporter; recipient }
+  let create ~status ~qualifier ~reporter ~recipient =
+    { status; qualifier; reporter; recipient }
   ;;
 
   let reporter t = t.reporter.txt
   let recipient t = Option.map t.recipient ~f:Loc.Txt.txt
-  let kind t = t.kind.txt
+  let status t = t.status.txt
   let qualifier t = t.qualifier.txt
+  let priority t = qualifier t |> Qualifier.priority
 
   (* Deprecated. *)
   let reported_by = reporter
   let for_ = recipient
+  let kind = status
 end
 
 module T = struct
@@ -185,7 +188,7 @@ let reindented_content ?(new_line_prefix = "") t =
           (* The len of the indentation is a heuristic in this case. *)
           start.pos_cnum - start.pos_bol + String.length t.comment_prefix + 1
         | Ok h ->
-          let start = Loc.start h.kind.loc in
+          let start = Loc.start h.status.loc in
           start.pos_cnum - start.pos_bol)
     in
     String.make len ' '
@@ -226,19 +229,19 @@ let reindented_content ?(new_line_prefix = "") t =
 
 let sort ts = List.sort ts ~compare:For_sorted_output.compare
 
-let kind t =
+let status t : Status.t =
   match t.header with
-  | Error _ -> Kind.CR
-  | Ok p -> Header.kind p
+  | Error _ -> CR
+  | Ok p -> Header.status p
 ;;
 
 let priority t : Priority.t =
   match t.header with
   | Error _ -> Now
   | Ok p ->
-    (match Header.kind p with
+    (match Header.status p with
      | XCR -> Now
-     | CR -> Header.qualifier p |> Qualifier.priority)
+     | CR -> Header.priority p)
 ;;
 
 let to_string t =
@@ -272,3 +275,4 @@ end
 (* Deprecated *)
 
 let work_on = priority
+let kind = status
diff --git a/lib/cr_comment/src/cr_comment0.mli b/lib/cr_comment/src/cr_comment0.mli
@@ -89,13 +89,17 @@ module Header : sig
       out, such as in [CR user1: Comment]. *)
   val recipient : t -> Vcs.User_handle.t option
 
-  val kind : t -> Kind.t
+  val status : t -> Status.t
 
   (** This returns the qualifier if present ([Soon] for [CR-soon] or [Someday]
       for [CR-someday]). If there is no qualifier specified, this returns
       [None]. *)
   val qualifier : t -> Qualifier.t
 
+  (** [priority t] represents the expectation as to when work on the CR is meant
+      to happen. It is based on the header's qualifier. *)
+  val priority : t -> Priority.t
+
   module With_loc : sig
     (** These getters allows you to access the position of each elements of the
         CR header. This is meant for tools processing CRs automatically, such
@@ -113,21 +117,25 @@ module Header : sig
     (** The location includes the entire keyword ["CR"] or ["XCR"] depending on
         the case. It stops right before the following char, that being a space
         or a ['-'] (and thus does not include it). *)
-    val kind : t -> Kind.t Loc.Txt.t
+    val status : t -> Status.t Loc.Txt.t
 
     (** When the CR is qualified as [Soon] or [Someday], the location returned
         starts right after the dash separator (but does not include it), and
         contains the entire qualifier keyword. For example, the location will
         include ["soon"] for a [CR-soon]. When the CR has no qualifier, there
         is no keyword to attach a location to : conventionally, we return
-        instead the location of the CR [kind] in this case. *)
+        instead the location of the CR [status] in this case. *)
     val qualifier : t -> Qualifier.t Loc.Txt.t
 
     (** {1 Deprecated}
 
         The following is deprecated and will be soon annotated as such with ocaml
         alerts. Please migrate, and do not use in new code. *)
 
+    (** This was renamed [status]. Hint: Run [ocamlmig migrate]. *)
+    val kind : t -> Status.t Loc.Txt.t
+    [@@migrate { repl = Rel.status }]
+
     (** This was renamed [reporter]. Hint: Run [ocamlmig migrate]. *)
     val reported_by : t -> Vcs.User_handle.t Loc.Txt.t
     [@@migrate { repl = Rel.reporter }]
@@ -138,7 +146,7 @@ module Header : sig
 
     (** This was renamed [qualifier]. Hint: Run [ocamlmig migrate]. *)
     val due : t -> Qualifier.t Loc.Txt.t
-    [@@migrate { repl = Rel.recipient }]
+    [@@migrate { repl = Rel.qualifier }]
   end
 
   (** {1 Deprecated}
@@ -153,6 +161,10 @@ module Header : sig
   (** This was renamed [recipient]. Hint: Run [ocamlmig migrate]. *)
   val for_ : t -> Vcs.User_handle.t option
   [@@migrate { repl = Rel.recipient }]
+
+  (** This was renamed [status]. Hint: Run [ocamlmig migrate]. *)
+  val kind : t -> Status.t
+  [@@migrate { repl = Rel.status }]
 end
 
 (** A [Cr_comment.t] is an immutable value holding the information and metadata
@@ -172,7 +184,7 @@ val content : t -> string
 val whole_loc : t -> Loc.t
 
 val header : t -> Header.t Or_error.t
-val kind : t -> Kind.t
+val status : t -> Status.t
 
 (** [priority t] represents the expectation as to when work on the CR is meant
     to happen. It is based on the header's qualifier except that XCRs and
@@ -228,7 +240,7 @@ module Private : sig
 
   module Header : sig
     val create
-      :  kind:Kind.t Loc.Txt.t
+      :  status:Status.t Loc.Txt.t
       -> qualifier:Qualifier.t Loc.Txt.t
       -> reporter:Vcs.User_handle.t Loc.Txt.t
       -> recipient:Vcs.User_handle.t Loc.Txt.t option
@@ -253,3 +265,7 @@ end
 (** This was renamed [priority]. Hint: Run [ocamlmig migrate]. *)
 val work_on : t -> Priority.t
 [@@migrate { repl = Rel.priority }]
+
+(** This was renamed [status]. Hint: Run [ocamlmig migrate]. *)
+val kind : t -> Status.t
+[@@migrate { repl = Rel.status }]
diff --git a/lib/cr_comment/src/filter.ml b/lib/cr_comment/src/filter.ml
@@ -57,7 +57,7 @@ let matches t ~cr =
     (match Cr_comment0.header cr with
      | Error _ -> false
      | Ok h ->
-       (match Cr_comment0.Header.kind h with
+       (match Cr_comment0.Header.status h with
         | XCR -> false
         | CR ->
           (match Cr_comment0.Header.qualifier h with
@@ -67,14 +67,14 @@ let matches t ~cr =
     (match Cr_comment0.header cr with
      | Error _ -> false
      | Ok h ->
-       (match Cr_comment0.Header.kind h with
+       (match Cr_comment0.Header.status h with
         | CR -> false
         | XCR -> true))
   | Now ->
     (match Cr_comment0.header cr with
      | Error _ -> false
      | Ok h ->
-       (match Cr_comment0.Header.kind h with
+       (match Cr_comment0.Header.status h with
         | XCR -> true
         | CR ->
           (match Cr_comment0.Header.qualifier h with
@@ -84,7 +84,7 @@ let matches t ~cr =
     (match Cr_comment0.header cr with
      | Error _ -> false
      | Ok h ->
-       (match Cr_comment0.Header.kind h with
+       (match Cr_comment0.Header.status h with
         | XCR -> false
         | CR ->
           (match Cr_comment0.Header.qualifier h with
@@ -94,7 +94,7 @@ let matches t ~cr =
     (match Cr_comment0.header cr with
      | Error _ -> false
      | Ok h ->
-       (match Cr_comment0.Header.kind h with
+       (match Cr_comment0.Header.status h with
         | XCR -> false
         | CR ->
           (match Cr_comment0.Header.qualifier h with
diff --git a/lib/cr_comment/src/priority.mli b/lib/cr_comment/src/priority.mli
@@ -22,7 +22,7 @@
 (** The [Priority.t] type represents a priority classification that can be
     associated with a code review comment (CR). Assignment of a priority is
     not always direct; rather, it is typically computed as a function of other
-    elements such as the CR's kind, review context, and qualifier.
+    elements such as the CR's status, review context, and qualifier.
 
     This classification is intended as a general convenience to help organize
     and filter CRs by indicating the reviewers's intent or suggested importance,
@@ -33,6 +33,10 @@
     at this level. Higher-level tools or code review systems built on top of CRs
     may define their own policies or behaviors around these categories.
 
+    In the terminology used in the project, you'll sometimes come across
+    language that makes a direct parallel between the CR's priority, and when
+    work on it is expected to be "due" or meant to happen.
+
     Typical interpretations:
     - [Now]: Should be addressed promptly (while working on a PR for example).
     - [Soon]: Should be addressed in the near future.
diff --git a/lib/cr_comment/src/status.ml b/lib/cr_comment/src/status.ml
diff --git a/lib/cr_comment/src/status.mli b/lib/cr_comment/src/status.mli
@@ -19,7 +19,7 @@
 (*_  <http://www.gnu.org/licenses/> and <https://spdx.org>, respectively.        *)
 (*_*******************************************************************************)
 
-(** The [Kind.t] type distinguishes between resolved and unresolved code review
+(** The [Status.t] type distinguishes between resolved and unresolved code review
     comments.
 
     - [CR]: An unresolved code review comment.
diff --git a/lib/cr_comment/test/test__status.ml b/lib/cr_comment/test/test__status.ml
@@ -20,17 +20,17 @@
 (********************************************************************************)
 
 let%expect_test "to_string" =
-  List.iter Cr_comment.Kind.all ~f:(fun kind ->
+  List.iter Cr_comment.Status.all ~f:(fun status ->
     print_s
       [%sexp
-        { kind : Cr_comment.Kind.t
-        ; to_string = (Cr_comment.Kind.to_string kind : string)
+        { status : Cr_comment.Status.t
+        ; to_string = (Cr_comment.Status.to_string status : string)
         }]);
   [%expect
     {|
-    ((kind      CR)
+    ((status    CR)
      (to_string CR))
-    ((kind      XCR)
+    ((status    XCR)
      (to_string XCR))
     |}];
   ()
diff --git a/lib/cr_comment/test/test__status.mli b/lib/cr_comment/test/test__status.mli
diff --git a/lib/crs_cli/src/annotation.ml b/lib/crs_cli/src/annotation.ml
@@ -77,7 +77,7 @@ let of_cr ~cr ~(config : Config.t) ~review_mode ~with_user_mentions =
     let title =
       match header with
       | Error _ -> "Invalid CR"
-      | Ok h -> Cr_comment.Kind.to_string (Cr_comment.Header.kind h)
+      | Ok h -> Cr_comment.Status.to_string (Cr_comment.Header.status h)
     in
     let assignee = Assignee.compute ~cr ~config ~review_mode in
     let with_user_mention =
@@ -97,7 +97,7 @@ let of_cr ~cr ~(config : Config.t) ~review_mode ~with_user_mentions =
         [ "This"
         ; (match header with
            | Error _ -> "invalid CR"
-           | Ok h -> Cr_comment.Kind.to_string (Cr_comment.Header.kind h))
+           | Ok h -> Cr_comment.Status.to_string (Cr_comment.Header.status h))
         ; "is"
         ; (match assignee.user with
            | None -> "unassigned"
diff --git a/lib/crs_cli/src/assignee.ml b/lib/crs_cli/src/assignee.ml
@@ -34,7 +34,7 @@ module Raw_assignee = struct
       (match Cr_comment.header cr with
        | Error _ -> Invalid
        | Ok h ->
-         (match Cr_comment.Header.kind h with
+         (match Cr_comment.Header.status h with
           | XCR -> Reporter (Cr_comment.Header.reporter h)
           | CR ->
             (match Cr_comment.Header.recipient h with
diff --git a/lib/crs_cli/src/cmd__tools__github__summary_comment.ml b/lib/crs_cli/src/cmd__tools__github__summary_comment.ml
@@ -76,7 +76,7 @@ This command is meant to be used to generate contents to include to the history
        then
          print_endline
            (Printf.sprintf
-              "Users with active CRs/XCRs: %s"
+              "Users with assigned CRs/XCRs: %s"
               (String.concat ~sep:", " assignees))
      in
      ())
diff --git a/lib/crs_cli/src/crs_cli.ml b/lib/crs_cli/src/crs_cli.ml
@@ -42,19 +42,19 @@ For more information, use the $(b,--help) flag on a subcommand.
           ~summary:"Useful utils to integrate with other tools."
           [ ( "config"
             , Command.group
-                ~summary:"Utils related to config files"
+                ~summary:"Utils related to config files."
                 [ "validate", Cmd__tools__config__validate.main ] )
           ; "emacs-grep", Cmd__tools__emacs_grep.main
           ; "enclosing-repo-info", Cmd__tools__enclosing_repo_info.main
           ; ( "github"
             , Command.group
-                ~summary:"Utils related to GitHub"
+                ~summary:"Utils related to GitHub."
                 [ "annotate-crs", Cmd__tools__github__annotate_crs.main
                 ; "summary-comment", Cmd__tools__github__summary_comment.main
                 ] )
           ; ( "reviewdog"
             , Command.group
-                ~summary:"Utils related to Reviewdog"
+                ~summary:"Utils related to Reviewdog."
                 [ "annotate-crs", Cmd__tools__reviewdog__annotate_crs.main ] )
           ] )
     ]
diff --git a/lib/crs_cli/src/summary_table.ml b/lib/crs_cli/src/summary_table.ml
@@ -92,7 +92,7 @@ module By_type = struct
       match Cr_comment.header cr with
       | Error _ -> Invalid
       | Ok h ->
-        (match Cr_comment.Header.kind h with
+        (match Cr_comment.Header.status h with
          | XCR -> XCR
          | CR ->
            (match Cr_comment.Header.qualifier h with
@@ -145,7 +145,7 @@ module Type = struct
     | Someday
 
   let of_header h =
-    match Cr_comment.Header.kind h with
+    match Cr_comment.Header.status h with
     | XCR -> XCR
     | CR ->
       (match Cr_comment.Header.qualifier h with
diff --git a/lib/crs_cli/test/test__annotation.ml b/lib/crs_cli/test/test__annotation.ml
@@ -47,7 +47,7 @@ let%expect_test "sexp_of_t" =
         (whole_loc _)
         (header (
           Ok (
-            (kind      CR)
+            (status    CR)
             (qualifier None)
             (reporter  user1)
             (recipient (user2)))))
diff --git a/lib/crs_parser/src/header_parser.ml b/lib/crs_parser/src/header_parser.ml
diff --git a/lib/crs_parser/test/test__crs_parser.ml b/lib/crs_parser/test/test__crs_parser.ml
diff --git a/test/cram/annotate-crs.t b/test/cram/annotate-crs.t
diff --git a/test/cram/grep.t b/test/cram/grep.t
diff --git a/test/expect/test__crs_rewriter.ml b/test/expect/test__crs_rewriter.ml
