Review 2

4 files changed, 32 insertions, 26 deletions

diff --git a/mod/mod-ci-github-gq.cxx b/mod/mod-ci-github-gq.cxx
index b112e5b..2afad81 100644
--- a/mod/mod-ci-github-gq.cxx
+++ b/mod/mod-ci-github-gq.cxx
@@ -426,9 +426,9 @@ namespace brep
 
   // Serialize a `createCheckRun` mutation for a build to GraphQL.
   //
-  // The conclusion argument (`co`) is required if the build_state is built
-  // because GitHub does not allow a check run status of completed without a
-  // conclusion.
+  // The conclusion argument (`co`) is required if the check run status is
+  // completed because GitHub does not allow a check run status of completed
+  // without a conclusion.
   //
   // The details URL argument (`du`) can be empty for queued but not for the
   // other states.
@@ -450,6 +450,10 @@ namespace brep
     //
     assert (!du || !du->empty ());
 
+    // Ensure we have conclusion if the status is completed.
+    //
+    assert (st != "COMPLETED" || co);
+
     ostringstream os;
 
     os << "mutation {"                                             << '\n';
@@ -494,8 +498,9 @@ namespace brep
 
   // Serialize an `updateCheckRun` mutation for one build to GraphQL.
   //
-  // The `br` argument is required if the build_state is built because GitHub
-  // does not allow updating a check run to completed without a conclusion.
+  // The `br` argument is required if the check run status is completed
+  // because GitHub does not allow updating a check run to completed without a
+  // conclusion.
   //
   // Throw invalid_argument if any of the arguments are invalid values (of
   // GraphQL types or otherwise).
@@ -507,6 +512,8 @@ namespace brep
                                 optional<timestamp> sa,     // Started at.
                                 optional<gq_built_result> br)
   {
+    assert (st != "COMPLETED" || br);
+
     ostringstream os;
 
     os << "mutation {"                                            << '\n'
@@ -588,7 +595,7 @@ namespace brep
   {
     // State cannot be built without a conclusion.
     //
-    assert (st != build_state::built);
+    assert (st != build_state::built && !ti.empty () && !su.empty ());
 
     string rq (
       gq_serialize_request (
diff --git a/mod/mod-ci-github-gq.hxx b/mod/mod-ci-github-gq.hxx
index 39d2c7e..0fc3817 100644
--- a/mod/mod-ci-github-gq.hxx
+++ b/mod/mod-ci-github-gq.hxx
@@ -49,7 +49,8 @@ namespace brep
   // Throw invalid_argument if the passed data is invalid, missing, or
   // inconsistent.
   //
-  // If the details_url is absent GitHub will use the app's homepage.
+  // If the details_url is absent GitHub will use the app's homepage. Title
+  // and summary are required and cannot be empty.
   //
   bool
   gq_create_check_run (const basic_mark& error,
@@ -59,13 +60,11 @@ namespace brep
                        const string& head_sha,
                        const optional<string>& details_url,
                        build_state,
-                       string title, string summary);
+                       string title,
+                       string summary);
 
-  // As above but create a check run in the built state.
-  //
-  // Note that a conclusion is required for the built state. And if a
-  // conclusion is provided with a queued or building state GitHub will change
-  // the state to built.
+  // As above but create a check run in the built state (which requires a
+  // conclusion).
   //
   struct gq_built_result
   {
diff --git a/mod/mod-ci-github-service-data.hxx b/mod/mod-ci-github-service-data.hxx
index ef30ffe..5d36696 100644
--- a/mod/mod-ci-github-service-data.hxx
+++ b/mod/mod-ci-github-service-data.hxx
@@ -33,19 +33,17 @@ namespace brep
 
     optional<result_status> status; // Only if state is built & synced.
 
-    // Note: never serialized (only used to pass information to the GraphQL
-    // functions).
+    // Note: these are never serialized (only used to pass information to the
+    // GraphQL functions).
     //
-    optional<string>        details_url;
-
-    // Note: never serialized (see details_url).
-    //
-    struct output_type
+    struct description_type
     {
       string title;
       string summary;
     };
-    optional<output_type>   output;
+
+    optional<string>           details_url;
+    optional<description_type> description;
 
     string
     state_string () const
diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx
index 45a9d14..a6fbe08 100644
--- a/mod/mod-ci-github.cxx
+++ b/mod/mod-ci-github.cxx
@@ -1936,10 +1936,12 @@ namespace brep
     // Create a synthetic check run with an in-progress state. Return the
     // check run on success or nullopt on failure.
     //
-    auto create_synthetic_cr = [iat,
-                                &sd, du = details_url (tenant_id),
-                                &error] (string name,
-                                         const check_run::output_type& output)
+    auto create_synthetic_cr = [&tenant_id,
+                                iat,
+                                &sd,
+                                &error,
+                                this] (string name,
+                                       const check_run::output_type& output)
       -> optional<check_run>
     {
       check_run cr;
@@ -1952,7 +1954,7 @@ namespace brep
                                iat->token,
                                sd.repository_node_id,
                                sd.report_sha,
-                               du,
+                               details_url (tenant_id),
                                build_state::building,
                                output.title, output.summary))
       {