diff options
Diffstat (limited to 'mod')
| -rw-r--r-- | mod/mod-ci-github-gh.cxx | 3 | ||||
| -rw-r--r-- | mod/mod-ci-github-gh.hxx | 16 | ||||
| -rw-r--r-- | mod/mod-ci-github-gq.cxx | 5 | ||||
| -rw-r--r-- | mod/mod-ci-github-service-data.hxx | 2 | ||||
| -rw-r--r-- | mod/mod-ci-github.cxx | 25 |
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 |