Post-review changes

5 files changed, 17 insertions, 15 deletions

diff --git a/mod/mod-ci-github-gh.cxx b/mod/mod-ci-github-gh.cxx
index 5664a7a..4e5ae9b 100644
--- a/mod/mod-ci-github-gh.cxx
+++ b/mod/mod-ci-github-gh.cxx
@@ -7,9 +7,8 @@
 
 namespace brep
 {
-  static const string gh_status[] {"QUEUED", "IN_PROGRESS", "COMPLETED"};
-
-  // Return the GitHub check run status corresponding to a build_state.
+  // Return the GitHub check run status corresponding to a build_state. Throw
+  // invalid_argument if the build_state value was invalid.
   //
   string
   gh_to_status (build_state st)
@@ -18,8 +17,11 @@ namespace brep
     //
     switch (st)
     {
-    case build_state::queued: return "QUEUED";
-      // @@ TODO: complete.
+    case build_state::queued:   return "QUEUED";
+    case build_state::building: return "IN_PROGRESS";
+    case build_state::built:    return "COMPLETED";
+    default:
+      throw invalid_argument ("invalid build_state value: " + to_string (st));
     }
   }
 
@@ -302,7 +304,7 @@ namespace brep
       };
 
       if      (c (tk, "token"))      token = p.next_expect_string ();
-      else if (c (ea, "expires_at")) expires_at = from_iso8601 (p.next_expect_string ());
+      else if (c (ea, "expires_at")) expires_at = gh_from_iso8601 (p.next_expect_string ());
       else p.next_expect_value_skip ();
     }
 
@@ -326,7 +328,7 @@ namespace brep
   }
 
   string
-  to_iso8601 (timestamp t)
+  gh_to_iso8601 (timestamp t)
   {
     return butl::to_string (t,
                             "%Y-%m-%dT%TZ",
@@ -335,7 +337,7 @@ namespace brep
   }
 
   timestamp
-  from_iso8601 (const string& s)
+  gh_from_iso8601 (const string& s)
   {
     return butl::from_string (s.c_str (), "%Y-%m-%dT%TZ", false /* local */);
   }
diff --git a/mod/mod-ci-github-gq.cxx b/mod/mod-ci-github-gq.cxx
index b9b47c3..cbf2b00 100644
--- a/mod/mod-ci-github-gq.cxx
+++ b/mod/mod-ci-github-gq.cxx
@@ -364,8 +364,6 @@ namespace brep
 
   // Serialize an `updateCheckRun` mutation for one build to GraphQL.
   //
-  // @@ TODO Support conclusion, output, etc.
-  //
   static string
   gq_mutation_update_check_run (const string& ri, // Repository ID.
                                 const string& ni, // Node ID.
diff --git a/mod/mod-ci-github-gq.hxx b/mod/mod-ci-github-gq.hxx
index fde93c4..2966670 100644
--- a/mod/mod-ci-github-gq.hxx
+++ b/mod/mod-ci-github-gq.hxx
@@ -53,6 +53,8 @@ namespace brep
   // with the new state. Return false and issue diagnostics if the request
   // failed.
   //
+  // @@ TODO Support conclusion, output, etc.
+  //
   bool
   gq_update_check_run (check_run& cr,
                        const string& installation_access_token,
diff --git a/mod/mod-ci-github-service-data.cxx b/mod/mod-ci-github-service-data.cxx
index 9f6d86d..e519936 100644
--- a/mod/mod-ci-github-service-data.cxx
+++ b/mod/mod-ci-github-service-data.cxx
@@ -31,7 +31,7 @@ namespace brep
     p.next_expect_member_object ("installation_access");
     installation_access.token = p.next_expect_member_string ("token");
     installation_access.expires_at =
-        from_iso8601 (p.next_expect_member_string ("expires_at"));
+        gh_from_iso8601 (p.next_expect_member_string ("expires_at"));
     p.next_expect (event::end_object);
 
     installation_id =
@@ -93,7 +93,7 @@ namespace brep
     //
     s.member_begin_object ("installation_access");
     s.member ("token", installation_access.token);
-    s.member ("expires_at", to_iso8601 (installation_access.expires_at));
+    s.member ("expires_at", gh_to_iso8601 (installation_access.expires_at));
     s.end_object ();
 
     s.member ("installation_id", installation_id);
diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx
index 3ff2330..27797f6 100644
--- a/mod/mod-ci-github.cxx
+++ b/mod/mod-ci-github.cxx
@@ -400,7 +400,7 @@ namespace brep
   //    showing the list of check runs, if the user is already on the previous
   //    check run's URL, nothing will automatically cause them to be
   //    redirected to the new URL. And so the user may sit on the abandoned
-  //    check run waiting forever for it to completed.)
+  //    check run waiting forever for it to be completed.)
   //
   //    As a result, we will deal with the out of order problem differently
   //    depending on the notification:
@@ -423,7 +423,7 @@ namespace brep
   //    time as queued/building is quite low (unlike queued and building).
   //
   //    Note also that with this semantics it's unlikely but possible that we
-  //    attempt to updat the service data in the wrong order. Specifically, it
+  //    attempt to update the service data in the wrong order. Specifically, it
   //    feels like this should not be possible in the ->building transition
   //    since we skip the building notification unless the check run in the
   //    service data is already in the queued state. But it is theoretically
@@ -439,7 +439,7 @@ namespace brep
   //    receive the reply).
   //
   //    In such cases, we record in the service data that the notification was
-  //    no synchronized and in subsequent notifications we do the best we can:
+  //    not synchronized and in subsequent notifications we do the best we can:
   //    if we have node_id, then we update, otherwise, we create (potentially
   //    overriding the check run created previously).
   //