Clean up

5 files changed, 12 insertions, 39 deletions

diff --git a/mod/mod-ci-github-gh.cxx b/mod/mod-ci-github-gh.cxx
index fc5cf82..2e13af2 100644
--- a/mod/mod-ci-github-gh.cxx
+++ b/mod/mod-ci-github-gh.cxx
@@ -368,14 +368,12 @@ namespace brep
       };
 
       if      (c (ni, "node_id"))        node_id = p.next_expect_string ();
-      else if (c (nm, "name"))           name = p.next_expect_string ();
       else if (c (fn, "full_name"))      path = p.next_expect_string ();
       else if (c (cu, "clone_url"))      clone_url = p.next_expect_string ();
       else p.next_expect_value_skip ();
     }
 
     if (!ni) missing_member (p, "gh_repository", "node_id");
-    if (!nm) missing_member (p, "gh_repository", "name");
     if (!fn) missing_member (p, "gh_repository", "full_name");
     if (!cu) missing_member (p, "gh_repository", "clone_url");
   }
@@ -384,7 +382,6 @@ namespace brep
   operator<< (ostream& os, const gh_repository& rep)
   {
     os << "node_id: " << rep.node_id
-       << ", name: " << rep.name
        << ", path: " << rep.path
        << ", clone_url: " << rep.clone_url;
 
diff --git a/mod/mod-ci-github-gh.hxx b/mod/mod-ci-github-gh.hxx
index eeda871..101e240 100644
--- a/mod/mod-ci-github-gh.hxx
+++ b/mod/mod-ci-github-gh.hxx
@@ -21,8 +21,6 @@ namespace butl
 
 namespace brep
 {
-   // @@@ Check if any data members are unused (once the dust settles).
-
   using build_queued_hints = tenant_service_build_queued::build_queued_hints;
 
   // GitHub request/response types (all start with gh_).
@@ -87,15 +85,22 @@ namespace brep
     string node_id;
     unsigned int number;
 
+    // Note that some of these base/head members are only used for logging.
+    //
     // @@ TMP The unused base/head members may be useful for trace output when
     //    we receive the pull_request webhook.
+    //
+    //    @@ Seems worth keeping. Example from logs:
+    //
+    //    base: { path: francoisk/libb2, ref: master, sha: 78b61bd3ce7b2b46bf7277446571d1ddd5b36a82 }
+    //    head: { path: francoisk/libb2, ref: dev,    sha: 32561c28ef039545d17d10bf10fad7babcf9c33a }
 
     string base_path; // Repository path (<org>/<repo>) under github.com.
-    string base_ref;  // @@ TODO Remove if remains unused.
-    string base_sha;  // @@ TODO Remove if remains unused.
+    string base_ref;
+    string base_sha;
 
     string head_path;
-    string head_ref; // @@ TODO Remove if remains unused.
+    string head_ref;
     string head_sha;
 
     explicit
@@ -133,7 +138,6 @@ namespace brep
   struct gh_repository
   {
     string node_id;
-    string name;
     string path; // Repository path (<org>/<repo>) under github.com.
     string clone_url;
 
diff --git a/mod/mod-ci-github-gq.cxx b/mod/mod-ci-github-gq.cxx
index 7b7e464..8f6bb16 100644
--- a/mod/mod-ci-github-gq.cxx
+++ b/mod/mod-ci-github-gq.cxx
@@ -273,11 +273,6 @@ namespace brep
             // Note that GitHub won't allow us to change a built check run to
             // any other state (but all other transitions are allowed).
             //
-            // @@ Are we handling the case where the resulting state (built)
-            //    differs from what we expect?
-            //
-            // @@@ Does built-to-built transition updates status?
-            //
             if (rst != st && rst != build_state::built)
             {
               error << "unexpected check_run status: received '" << rcr.status
diff --git a/mod/mod-ci-github-service-data.hxx b/mod/mod-ci-github-service-data.hxx
index 776ec8d..32d5917 100644
--- a/mod/mod-ci-github-service-data.hxx
+++ b/mod/mod-ci-github-service-data.hxx
@@ -11,8 +11,6 @@
 
 namespace brep
 {
-  // @@@ Check if any data members are unused (once the dust settles).
-
   // Service data associated with the tenant (corresponds to GH check suite).
   //
   // It is always a top-level JSON object and the first member is always the
diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx
index 113da2e..e04570e 100644
--- a/mod/mod-ci-github.cxx
+++ b/mod/mod-ci-github.cxx
@@ -21,19 +21,6 @@
 
 // @@ Remaining TODOs
 //
-//    - Rerequested checks
-//
-//      - check_suite (action: rerequested): received when user re-runs all
-//        checks.
-//
-//      - check_run (action: rerequested): received when user re-runs a
-//        specific check or all failed checks.
-//
-//        @@ TMP I have confirmed that the above is accurate.
-//
-//      Will need to extract a few more fields from check_runs, but the layout
-//      is very similar to that of check_suite.
-//
 //    - Choose strong webhook secret (when deploying).
 //
 //    - Check that delivery UUID has not been received before (replay attack).
@@ -280,9 +267,6 @@ namespace brep
     // is that we want be "notified" of new actions at which point we can
     // decide whether to ignore them or to handle.
     //
-    // @@ There is also check_run even (re-requested by user, either
-    //    individual check run or all the failed check runs).
-    //
     if (event == "check_suite")
     {
       gh_check_suite_event cs;
@@ -558,10 +542,6 @@ namespace brep
 
     l3 ([&]{trace << "installation_access_token { " << *iat << " }";});
 
-    // @@ What happens if we call this functions with an already existing
-    //    node_id (e.g., replay attack). See the UUID header above.
-    //
-
     // While it would have been nice to cancel CIs of PRs with this branch as
     // base not to waste resources, there are complications: Firstly, we can
     // only do this for remote PRs (since local PRs will most likely share the
@@ -2253,9 +2233,8 @@ namespace brep
 
     NOTIFICATION_DIAG (log_writer);
 
-    // @@ TODO Include service_data::event_node_id and perhaps ts.id in
-    //    diagnostics? E.g. when failing to update check runs we print the
-    //    build ID only.
+    // @@ TODO Include ts.id in diagnostics? Check run build ids alone seem
+    //    kind of meaningless. Log lines get pretty long this way however.
 
     service_data sd;
     try