Skip to content

Commit

Permalink
Merge pull request #104 from terrateamio/pro-624-add-apply-requiremen…
Browse files Browse the repository at this point in the history
…ts-ready-to-review

PRO-624 ADD Apply requirement for draft pr being ready to review
  • Loading branch information
orbitz authored Nov 26, 2024
2 parents 7f478cd + d23cf77 commit 63729f7
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 14 deletions.
4 changes: 4 additions & 0 deletions api_schemas/terrat/config-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@
"merge_conflicts": {
"$ref": "#/definitions/apply-requirements-checks-merge-conflicts"
},
"require_ready_for_review_pr": {
"default": true,
"type": "boolean"
},
"status_checks": {
"$ref": "#/definitions/apply-requirements-checks-status-checks"
},
Expand Down
16 changes: 9 additions & 7 deletions code/src/terrat/terrat_evaluator3.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1988,13 +1988,15 @@ module Make (S : S) = struct
let open Abb.Future.Infix_monad in
Abbs_time_it.run
(fun time ->
Logs.info (fun m ->
m
"EVALUATOR : %s : DV : PULL_REQUEST : repo=%s : pull_number=%d : time=%f"
state.State.request_id
(S.Repo.to_string repo)
pull_request_id
time))
(* This is pretty noisy, so only log if the value is greater than 0. *)
if time > 0.0 then
Logs.info (fun m ->
m
"EVALUATOR : %s : DV : PULL_REQUEST : repo=%s : pull_number=%d : time=%f"
state.State.request_id
(S.Repo.to_string repo)
pull_request_id
time))
(fun () ->
Cache.Pull_request.fetch
Cache.pull_request
Expand Down
31 changes: 27 additions & 4 deletions code/src/terrat/terrat_github_evaluator3.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1093,6 +1093,7 @@ struct
match_ : Terrat_change_match3.Dirspace_config.t;
merge_conflicts : bool option;
passed : bool;
ready_for_review : bool option;
status_checks : bool option;
status_checks_failed : Terrat_commit_check.t list;
}
Expand Down Expand Up @@ -3454,6 +3455,10 @@ struct
bool (CCOption.is_some ar.Ar.merge_conflicts) );
( "merge_conflicts_check",
bool (CCOption.get_or ~default:false ar.Ar.merge_conflicts) );
( "ready_for_review_enabled",
bool (CCOption.is_some ar.Ar.ready_for_review) );
( "ready_for_review_check",
bool (CCOption.get_or ~default:false ar.Ar.ready_for_review) );
("status_checks_enabled", bool (CCOption.is_some ar.Ar.status_checks));
( "status_checks_check",
bool (CCOption.get_or ~default:false ar.Ar.status_checks) );
Expand Down Expand Up @@ -4871,7 +4876,14 @@ struct
Terrat_tag_query.match_ ~ctx ~tag_set:tags tag_query)
checks
with
| Some { Abc.tag_query; merge_conflicts; status_checks; approved; _ } ->
| Some
{
Abc.tag_query;
merge_conflicts;
status_checks;
approved;
require_ready_for_review_pr;
} ->
compute_approved request_id access_control_ctx approved approved_reviews
>>= fun approved_result ->
let ignore_matching = status_checks.Sc.ignore_matching in
Expand Down Expand Up @@ -4904,11 +4916,15 @@ struct
| St.Merged _ -> true
| St.Open _ | St.Closed -> false
in
let ready_for_review =
(not require_ready_for_review_pr) || not (Pull_request.is_draft_pr pull_request)
in
let passed =
merged
|| ((not approved.Ac.enabled) || approved_result)
&& ((not merge_conflicts.Mc.enabled) || merge_result)
&& ((not status_checks.Sc.enabled) || all_commit_check_success)
&& ready_for_review
in
let apply_requirements =
{
Expand All @@ -4917,6 +4933,8 @@ struct
approved = (if approved.Ac.enabled then Some approved_result else None);
merge_conflicts =
(if merge_conflicts.Mc.enabled then Some merge_result else None);
ready_for_review =
(if require_ready_for_review_pr then Some ready_for_review else None);
status_checks =
(if status_checks.Sc.enabled then Some all_commit_check_success else None);
status_checks_failed = failed_commit_checks;
Expand All @@ -4926,22 +4944,26 @@ struct
Logs.info (fun m ->
m
"GITHUB_EVALUATOR : %s : APPLY_REQUIREMENTS_CHECKS : tag_query=%s \
approved=%s merge_conflicts=%s status_checks=%s"
approved=%s merge_conflicts=%s status_checks=%s \
require_ready_for_review=%s"
request_id
(Terrat_tag_query.to_string tag_query)
(Bool.to_string approved.Ac.enabled)
(Bool.to_string merge_conflicts.Mc.enabled)
(Bool.to_string status_checks.Sc.enabled));
(Bool.to_string status_checks.Sc.enabled)
(Bool.to_string require_ready_for_review_pr));
Logs.info (fun m ->
m
"GITHUB_EVALUATOR : %s : APPLY_REQUIREMENTS_RESULT : tag_query=%s \
approved=%s merge_check=%s commit_check=%s merged=%s passed=%s"
approved=%s merge_check=%s commit_check=%s merged=%s ready_for_review=%s \
passed=%s"
request_id
(Terrat_tag_query.to_string tag_query)
(Bool.to_string approved_result)
(Bool.to_string merge_result)
(Bool.to_string all_commit_check_success)
(Bool.to_string merged)
(Bool.to_string ready_for_review)
(Bool.to_string passed));
Abb.Future.return (Ok apply_requirements)
| None ->
Expand All @@ -4952,6 +4974,7 @@ struct
match_;
approved = None;
merge_conflicts = None;
ready_for_review = None;
status_checks = None;
status_checks_failed = [];
approved_reviews = [];
Expand Down
23 changes: 20 additions & 3 deletions code/src/terrat_base_repo_config_v1/terrat_base_repo_config_v1.ml
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ module Apply_requirements = struct
type t = {
approved : Approved.t; [@default Approved.make ()]
merge_conflicts : Merge_conflicts.t; [@default Merge_conflicts.make ()]
require_ready_for_review_pr : bool; [@default true]
status_checks : Status_checks.t; [@default Status_checks.make ()]
tag_query : Tag_query.t; [@default Terrat_tag_query.any]
}
Expand Down Expand Up @@ -851,7 +852,7 @@ let of_version_1_apply_requirements_checks =
let open CCResult.Infix in
let module I = C2.Items in
CCResult.map_l
(fun { I.approved; merge_conflicts; status_checks; tag_query } ->
(fun { I.approved; merge_conflicts; require_ready_for_review_pr; status_checks; tag_query } ->
CCResult.map_err
(function
| `Tag_query_error err -> `Apply_requirements_check_tag_query_err err)
Expand All @@ -867,7 +868,14 @@ let of_version_1_apply_requirements_checks =
>>= fun merge_conflicts ->
map_opt get_apply_requirements_checks_status_checks status_checks
>>= fun status_checks ->
Ok (Ar.Check.make ~tag_query ?approved ?merge_conflicts ?status_checks ()))
Ok
(Ar.Check.make
~require_ready_for_review_pr
~tag_query
?approved
?merge_conflicts
?status_checks
()))
checks

let of_version_1_file_patterns fp = CCResult.map_l File_pattern.make fp
Expand Down Expand Up @@ -1696,10 +1704,19 @@ let to_version_1_apply_requirements_status_checks sc =
let to_version_1_apply_requirements_checks =
let module C2 = Terrat_repo_config.Apply_requirements_checks_2 in
CCList.map
(fun { Apply_requirements.Check.approved; merge_conflicts; status_checks; tag_query } ->
(fun
{
Apply_requirements.Check.approved;
merge_conflicts;
require_ready_for_review_pr;
status_checks;
tag_query;
}
->
{
C2.Items.approved = Some (to_version_1_apply_requirements_approved approved);
merge_conflicts = Some (to_version_1_apply_requirements_merge_conflicts merge_conflicts);
require_ready_for_review_pr;
status_checks = Some (to_version_1_apply_requirements_status_checks status_checks);
tag_query = Terrat_tag_query.to_string tag_query;
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ module Apply_requirements : sig
type t = {
approved : Approved.t; [@default Approved.make ()]
merge_conflicts : Merge_conflicts.t; [@default Merge_conflicts.make ()]
require_ready_for_review_pr : bool; [@default true]
status_checks : Status_checks.t; [@default Status_checks.make ()]
tag_query : Tag_query.t; [@default Terrat_tag_query.any]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
@?merge_conflicts_enabled-@
**Merge Conflicts**: @?merge_conflicts_check@:thumbsup: Passed@/merge_conflicts_check@@!merge_conflicts_check@:heavy_multiplication_x: Failed@/merge_conflicts_check@
@/merge_conflicts_enabled-@
@?ready_for_review_enabled-@
**Ready for Review**: @?ready_for_review_check@:thumbsup: Passed@/ready_for_review_check@@!ready_for_review_check@:heavy_multiplication_x: Failed - Pull request still in draft stage.@/ready_for_review_check@
@/ready_for_review_enabled-@
@?status_checks_enabled-@
**Status Checks**: @?status_checks_check@:thumbsup: Passed@/status_checks_check@@!status_checks_check@:heavy_multiplication_x: Failed@/status_checks_check@
@!status_checks_check-@
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ module Items = struct
approved : Terrat_repo_config_apply_requirements_checks_approved_2.t option; [@default None]
merge_conflicts : Terrat_repo_config_apply_requirements_checks_merge_conflicts.t option;
[@default None]
require_ready_for_review_pr : bool; [@default true]
status_checks : Terrat_repo_config_apply_requirements_checks_status_checks.t option;
[@default None]
tag_query : string;
Expand Down

0 comments on commit 63729f7

Please sign in to comment.