diff options
| -rw-r--r-- | mod/mod-ci-github-gq.cxx | 27 | ||||
| -rw-r--r-- | mod/mod-ci-github.cxx | 7 | ||||
| -rw-r--r-- | mod/mod-ci-github.hxx | 8 |
3 files changed, 32 insertions, 10 deletions
diff --git a/mod/mod-ci-github-gq.cxx b/mod/mod-ci-github-gq.cxx index 26b4c1f..df81854 100644 --- a/mod/mod-ci-github-gq.cxx +++ b/mod/mod-ci-github-gq.cxx @@ -628,9 +628,13 @@ namespace brep { if (ma == "CONFLICTING") value = ""; - else if (ma == "UNKNOWN") + if (ma == "UNKNOWN") + ; // Still being generated; leave value absent. + else { - // Still being generated; leave value absent. + error << "unexpected mergeable value '" << ma << "'"; + + // Carry on as if it were UNKNOWN. } // Skip the merge commit ID (which should be null). @@ -697,7 +701,18 @@ namespace brep // page) open pull requests with the specified base branch from the // repository with the specified node ID. // - // @@ TMP Should we support more than 100? + // @@ TMP Should we support more/less than 100? + // + // Doing more (or even 100) could waste a lot of CI resources on + // re-testing stale PRs. Maybe we should create a failed synthetic + // conclusion check run asking the user to re-run the CI manually if/when + // needed. + // + // Note that we cannot request more than 100 at a time (will need to + // do multiple requests with paging, etc). + // + // Also, maybe we should limit the result to "fresh" PRs, e.g., those + // that have been "touched" in the last week. // // Example query: // @@ -789,7 +804,7 @@ namespace brep { using event = json::event; - gq_parse_response (p, [this] (json::parser& p) + auto parse_data = [this] (json::parser& p) { p.next_expect (event::begin_object); @@ -825,7 +840,9 @@ namespace brep } p.next_expect (event::end_object); - }); + }; + + gq_parse_response (p, move (parse_data)); } resp () = default; diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx index 8ea730f..70c1c7d 100644 --- a/mod/mod-ci-github.cxx +++ b/mod/mod-ci-github.cxx @@ -495,7 +495,8 @@ namespace brep // The merge commits of any open pull requests with this branch as base // branch will now be out of date, and thus so will be their CI builds and - // associated check runs. + // associated check runs (and, no, GitHub does not invalidate those CI + // results automatically; see below). // // Unfortunately GitHub does not provide a webhook for PR base branch // updates (as it does for PR head branch updates) so we have to handle it @@ -512,7 +513,9 @@ namespace brep // user would be able to merge a broken PR. // // Regardless of the nature of the error, we have to let the check suite - // handling code proceed so we only issue diagnostics. + // handling code proceed so we only issue diagnostics. Note also that we + // want to run this code as early as possible to minimize the window of + // the user seeing misleading CI results. // { // Fetch open pull requests with the check suite's head branch as base diff --git a/mod/mod-ci-github.hxx b/mod/mod-ci-github.hxx index 07295f2..489aac7 100644 --- a/mod/mod-ci-github.hxx +++ b/mod/mod-ci-github.hxx @@ -17,6 +17,8 @@ namespace brep { + struct service_data; + class ci_github: public database_module, private ci_start, public tenant_service_build_unloaded, @@ -83,8 +85,8 @@ namespace brep // true, cancel its existing CI request first. // // Return true if an unloaded CI request was created. Ignore failure to - // cancel because the CI request may already have been cancelled for a - // legitimate reason. + // cancel because the CI request may already have been cancelled for other + // reasons. // // After this call we will start getting the build_unloaded() // notifications until (1) we load the request, (2) we cancel it, or (3) @@ -94,7 +96,7 @@ namespace brep create_pull_request_ci (const basic_mark& error, const basic_mark& warn, const basic_mark& trace, - string service_data, + const service_data&, const string& pull_request_node_id, bool cancel_first) const; |