aboutsummaryrefslogtreecommitdiff
path: root/mod
diff options
context:
space:
mode:
authorFrancois Kritzinger <francois@codesynthesis.com>2024-05-30 10:18:31 +0200
committerFrancois Kritzinger <francois@codesynthesis.com>2024-06-05 09:12:46 +0200
commit1a5faed8c5d713ebb4b0ece9f8111fe8a255b42b (patch)
treede04e3b09f5f67226713aeec556d46d5a42ebd0d /mod
parent2e2a86685fb752b7a6a0b6b6aa0764beb1bdde8a (diff)
Post-review changes
Diffstat (limited to 'mod')
-rw-r--r--mod/mod-ci-github-service-data.cxx2
-rw-r--r--mod/mod-ci-github.cxx100
2 files changed, 68 insertions, 34 deletions
diff --git a/mod/mod-ci-github-service-data.cxx b/mod/mod-ci-github-service-data.cxx
index 1121f2f..3af0c71 100644
--- a/mod/mod-ci-github-service-data.cxx
+++ b/mod/mod-ci-github-service-data.cxx
@@ -108,7 +108,7 @@ namespace brep
timestamp iat_ea,
uint64_t iid,
string rid,
- string hs,
+ string rs,
string rcu,
uint32_t prn)
: warning_success (ws),
diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx
index 16a8cc0..0bcaa3d 100644
--- a/mod/mod-ci-github.cxx
+++ b/mod/mod-ci-github.cxx
@@ -357,7 +357,25 @@ namespace brep
throw invalid_request (400, move (m));
}
- return handle_pull_request (move (pr), warning_success);
+ if (pr.action == "opened" || pr.action == "synchronize")
+ {
+ // opened: A pull request was opened.
+ //
+ // synchronize: A pull request's head branch was updated from the base
+ // branch or new commits were pushed to the head
+ // branch. (Note that there is no equivalent event for
+ // the base branch. That case gets handled in
+ // handle_check_suite_request() instead.)
+ //
+ return handle_pull_request (move (pr), warning_success);
+ }
+ else
+ {
+ // Ignore the remaining actions by sending a 200 response with empty
+ // body.
+ //
+ return true;
+ }
}
else
{
@@ -536,8 +554,6 @@ namespace brep
l3 ([&]{trace << "pull_request event { " << pr << " }";});
- // @@ TODO: check action, ignore those we don't care about.
-
// While we don't need the installation access token in this request,
// let's obtain it to flush out any permission issues early. Also, it is
// valid for an hour so we will most likely make use of it.
@@ -651,7 +667,7 @@ namespace brep
iat = &sd.installation_access;
if (iat == nullptr)
- return nullptr;
+ return nullptr; // Try again on the next call.
// Create an in-progress check run. Return the check run on success or
// nullopt on failure.
@@ -666,8 +682,7 @@ namespace brep
iat->token,
sd.repository_node_id,
sd.report_sha,
- // @@ TODO What details URL to use? Omit.
- "https://build2.org", // details URL.
+ nullopt, // details_url
build_state::building))
{
return cr;
@@ -676,33 +691,31 @@ namespace brep
return nullopt;
};
- // Update the merge check run with success or failure. Return the check
- // run on success or nullopt on failure.
- //
- // @@ Make update_cr;
+ // Update a check run with success or failure. Return the check run on
+ // success or nullopt on failure.
//
- auto update_merge_cr = [iat,
- &merge_node_id,
- &error] (result_status rs,
- const string& summary)
+ auto update_cr = [iat, &sd, &error] (const string& node_id,
+ const string& name,
+ result_status rs,
+ string summary)
-> optional<check_run>
{
- assert (!merge_id.empty ());
+ assert (!node_id.empty ());
optional<gq_built_result> br (
gq_built_result (gh_to_conclusion (rs, sd.warning_success),
circle (rs) + ' ' + ucase (to_string (rs)),
- summary));
+ move (summary)));
check_run cr;
- cr.name = "merge-commit"; // For display purposes only.
+ cr.name = name; // For display purposes only.
if (gq_update_check_run (error,
cr,
iat->token,
sd.repository_node_id,
- merge_node_id,
- "https://build2.org", // details URL.
+ node_id,
+ nullopt, // details_url
build_state::built,
move (br)))
{
@@ -734,12 +747,15 @@ namespace brep
//
if (first)
{
- if (auto cr = create_cr ("merge-commit"))
+ if (auto cr = create_cr (merge_check_run_name))
{
l3 ([&]{trace << "created check_run { " << *cr << " }";});
- merge_node_id = move (cr->node_id);
+ merge_node_id = move (*cr->node_id);
}
+ // @@ TMP If new_iat.has_value() then we won't store the new IAT. Same
+ // applies to some of the other cases below.
+ //
else
return nullptr; // Try again on the next call.
}
@@ -772,12 +788,22 @@ namespace brep
// failed synthetic conclusion check run since the PR cannot be merged
// anyway.
- if (auto cr = update_merge_cr (result_status::error,
- "merge would create conflicts"))
+ if (auto cr = update_cr (merge_node_id, merge_check_run_name,
+ result_status::error,
+ "Merge would create conflicts"))
{
l3 ([&]{trace << "updated check_run { " << *cr << " }";});
- // @@ TODO: cancel CI request
+ // Cancel the CI request.
+ //
+ if (!cancel (error, warn, &trace, *build_db_, ts.type, ts.id))
+ {
+ // Nothing we can do but also highly unlikely.
+ //
+ error << "unable to cancel CI request: "
+ << "no tenant for service type/ID "
+ << ts.type << '/' << ts.id;
+ }
return nullptr; // No need to update service data in this case.
}
@@ -848,31 +874,33 @@ namespace brep
if (second)
{
- if (auto cr = create_cr ("conclusion"))
+ if (auto cr = create_cr (conclusion_check_run_name))
{
l3 ([&]{trace << "created check_run { " << *cr << " }";});
- conclusion_node_id = move (cr->node_id);
+ conclusion_node_id = move (*cr->node_id);
}
}
else
- conclusion_node_id = sd.conclusion_node_id;
+ conclusion_node_id = *sd.conclusion_node_id;
if (!conclusion_node_id.empty ()) // Conclusion check run was created.
{
// Update merge check run to successful.
//
- if (auto cr = update_merge_cr (result_status::success,
- "merge would succeed"))
+ if (auto cr = update_cr (merge_node_id, merge_check_run_name,
+ result_status::success, "Merge would succeed"))
{
l3 ([&]{trace << "updated check_run { " << *cr << " }";});
// Load the CI request.
//
- // @@ TODO: commit id.
+ // Example repository URL fragment:
+ //
+ // #pull/28/merge@1b6c9a361086ed93e6f1e67189e82d52de91c49b
//
- repository_location rl (*sd.repository_clone_url + "#refs/pull/" +
- to_string (*sd.pr_number) + "/merge",
+ repository_location rl (*sd.repository_clone_url + "#pull/" +
+ to_string (*sd.pr_number) + "/merge@" + *mc,
repository_type::git);
optional<start_result> r (
@@ -886,7 +914,13 @@ namespace brep
else msg += "Internal service error";
msg += "\n```";
- // @@ TODO: fail conclusion check run.
+ if (auto cr = update_cr (conclusion_node_id,
+ conclusion_check_run_name,
+ result_status::error,
+ move (msg)))
+ {
+ l3 ([&]{trace << "updated check_run { " << *cr << " }";});
+ }
}
}
else