diff options
Diffstat (limited to 'mod')
| -rw-r--r-- | mod/mod-ci-github.cxx | 33 |
1 files changed, 20 insertions, 13 deletions
diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx index 7ede14f..ea16fda 100644 --- a/mod/mod-ci-github.cxx +++ b/mod/mod-ci-github.cxx @@ -406,8 +406,7 @@ namespace brep // 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. @@ Not anymore.) + // no equivalent event for the base branch.) // // Note that both cases are handled the same: we start a new CI // request which will be reported on the new commit id. @@ -507,11 +506,13 @@ namespace brep // // While it would have been nice to cancel CIs of PRs with this branch as - // base not to waste resources, there are complications: Firsty, we can - // only do this for remote PRs (since local PRs may share the result with - // branch push). Secondly, we try to do our best even if the branch - // protection rule for head behind is not enabled. In this case, it would - // be good to complete the CI. So maybe/later. + // 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 + // result with branch push). Secondly, we try to do our best even if the + // branch protection rule for head behind is not enabled. In this case, it + // would be good to complete the CI. So maybe/later. See also the head + // case in handle_pull_request(), where we do cancel remote PRs that are + // not shared. // Service id that uniquely identifies the CI tenant. // @@ -1480,12 +1481,6 @@ namespace brep l3 ([&]{trace << "installation_access_token { " << *iat << " }";}); - // Note that similar to the branch push case above, while it would have - // been nice to cancel the previous CI job once the PR head moves (the - // "synchronize" event), due to the head sharing problem the previous CI - // job might actually still be relevant (in both local and remote PR - // cases). - // Distinguish between local and remote PRs by comparing the head and base // repositories' paths. // @@ -1494,6 +1489,18 @@ namespace brep ? service_data::local : service_data::remote); + // Note that similar to the branch push case above, while it would have + // been nice to cancel the previous CI job once the PR head moves (the + // "synchronize" event), due to the head sharing problem the previous CI + // job might actually still be relevant (in both local and remote PR + // cases). So we only do it for the remote PRs and only if the head is not + // shared (via tenant reference counting). + // + if (kind == service_data::remote && pr.action == "synchronize") + { + // @@ TODO: call cancel on old SHA with refcount=true. + } + // Note: for remote PRs the check_sha will be set later, in // build_unloaded_pre_check(), to test merge commit id. // |