From 75965979e68831b46cfde18a0aee51a7d63119e3 Mon Sep 17 00:00:00 2001 From: Francois Kritzinger Date: Fri, 10 May 2024 14:15:25 +0200 Subject: Post-review changes --- mod/mod-ci-github-gq.cxx | 9 ++------- mod/mod-ci-github-service-data.cxx | 6 +++--- mod/mod-ci-github-service-data.hxx | 7 +++---- mod/mod-ci-github.cxx | 30 +++++++++++++++--------------- mod/module.cli | 3 --- 5 files changed, 23 insertions(+), 32 deletions(-) (limited to 'mod') diff --git a/mod/mod-ci-github-gq.cxx b/mod/mod-ci-github-gq.cxx index bea9a7f..954f5a8 100644 --- a/mod/mod-ci-github-gq.cxx +++ b/mod/mod-ci-github-gq.cxx @@ -80,9 +80,7 @@ namespace brep // serialize data to a stringstream and only parse it later once we're // sure there are no errors. // - ostringstream data; // The value of the data field. - - // @@@ Use iostringstream for both output and input. + stringstream data; // The value of the data field. p.next_expect (event::begin_object); @@ -144,8 +142,7 @@ namespace brep // Parse the data field now that we know there are no errors. // - string d (data.str ()); - json::parser dp (d, p.input_name); + json::parser dp (data, p.input_name); parse_data (dp); } @@ -183,8 +180,6 @@ namespace brep // } // } // - // @@@ TODO Handle response errors properly. - // static vector gq_parse_response_check_runs (json::parser& p) { diff --git a/mod/mod-ci-github-service-data.cxx b/mod/mod-ci-github-service-data.cxx index 83e952b..f1d5fd5 100644 --- a/mod/mod-ci-github-service-data.cxx +++ b/mod/mod-ci-github-service-data.cxx @@ -38,7 +38,7 @@ namespace brep installation_id = p.next_expect_member_number ("installation_id"); - repository_id = p.next_expect_member_string ("repository_id"); + repository_node_id = p.next_expect_member_string ("repository_id"); head_sha = p.next_expect_member_string ("head_sha"); p.next_expect_member_array ("check_runs"); @@ -75,7 +75,7 @@ namespace brep : warning_success (ws), installation_access (move (iat_tok), iat_ea), installation_id (iid), - repository_id (move (rid)), + repository_node_id (move (rid)), head_sha (move (hs)) { } @@ -100,7 +100,7 @@ namespace brep s.end_object (); s.member ("installation_id", installation_id); - s.member ("repository_id", repository_id); + s.member ("repository_id", repository_node_id); s.member ("head_sha", head_sha); s.member_begin_array ("check_runs"); diff --git a/mod/mod-ci-github-service-data.hxx b/mod/mod-ci-github-service-data.hxx index ff1c1a3..c4e20b3 100644 --- a/mod/mod-ci-github-service-data.hxx +++ b/mod/mod-ci-github-service-data.hxx @@ -56,9 +56,8 @@ namespace brep gh_installation_access_token installation_access; uint64_t installation_id; - // @@@ TODO Rename to repository_node_id. - // - string repository_id; // GitHub-internal opaque repository id. + + string repository_node_id; // GitHub-internal opaque repository id. string head_sha; @@ -81,7 +80,7 @@ namespace brep string iat_token, timestamp iat_expires_at, uint64_t installation_id, - string repository_id, + string repository_node_id, string head_sha); service_data () = default; diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx index 5aa4e6d..a99e516 100644 --- a/mod/mod-ci-github.cxx +++ b/mod/mod-ci-github.cxx @@ -41,16 +41,20 @@ // Resources: // +// Creating an App: +// https://docs.github.com/en/apps/creating-github-apps/about-creating-github-apps/best-practices-for-creating-a-github-app +// // Webhooks: // https://docs.github.com/en/webhooks/using-webhooks/best-practices-for-using-webhooks // https://docs.github.com/en/webhooks/using-webhooks/validating-webhook-deliveries // // REST API: -// https://docs.github.com/en/rest/using-the-rest-api/best-practices-for-using-the-rest-api?apiVersion=2022-11-28 -// @@@ Add link to GraphQL? +// All docs: https://docs.github.com/en/rest#all-docs +// Best practices: https://docs.github.com/en/rest/using-the-rest-api/best-practices-for-using-the-rest-api +// +// GraphQL API: +// Reference: https://docs.github.com/en/graphql/reference // -// Creating an App: -// https://docs.github.com/en/apps/creating-github-apps/about-creating-github-apps/best-practices-for-creating-a-github-app using namespace std; using namespace butl; @@ -109,8 +113,6 @@ namespace brep // Process headers. // - // @@@ TMP Shouldn't we also error<< in some of these header problem cases? - // string event; // Webhook event. string hmac; // Received HMAC. { @@ -529,12 +531,11 @@ namespace brep } else if (*istate == build_state::built) { - // Unexpectd built->queued transition (rebuild). + // Unexpected built->queued transition (rebuild). // warn << "check run " << bid << ": unexpected rebuild"; } - else - ; // Ignore interrupted. + else {} // Ignore interrupted. } else { @@ -583,7 +584,7 @@ namespace brep if (gq_create_check_runs (error, crs, iat->token, - sd.repository_id, sd.head_sha, + sd.repository_node_id, sd.head_sha, build_state::queued)) { for (const check_run& cr: crs) @@ -677,8 +678,7 @@ namespace brep cr = move (*scr); cr->state_synced = false; } - else - ; // Network error during queued notification, ignore. + else {} // Network error during queued notification, ignore. } else warn << "check run " << bid << ": out of order building " @@ -719,7 +719,7 @@ namespace brep if (gq_update_check_run (error, *cr, iat->token, - sd.repository_id, + sd.repository_node_id, *cr->node_id, details_url (b), build_state::building)) @@ -990,7 +990,7 @@ namespace brep if (gq_update_check_run (error, cr, iat->token, - sd.repository_id, + sd.repository_node_id, *cr.node_id, details_url (b), build_state::built, @@ -1014,7 +1014,7 @@ namespace brep if (gq_create_check_run (error, cr, iat->token, - sd.repository_id, + sd.repository_node_id, sd.head_sha, details_url (b), build_state::built, diff --git a/mod/module.cli b/mod/module.cli index 1b4b445..3b47aec 100644 --- a/mod/module.cli +++ b/mod/module.cli @@ -815,9 +815,6 @@ namespace brep { }; - // @@@ TODO Is etc/brep-module.conf updated manually? Yes, will need to - // replicate there eventually. - // class ci_github: repository_url, ci_start, ci_cancel, build, build_db, -- cgit v1.1