Skip to content

Commit 397b61e

Browse files
committed
Merge branch 'yasu/projectowners'
* yasu/projectowners: docs: describe draft PR behavior github: don't request reviews if no diff from current reviewer list github: request project owner review if non-draft only cfg: change project owner syntax to record list docs: + project_owners github: request review when pr labeled cfg: + project_owners github: derive project owners from incoming label list github: + request_reviewers api
2 parents 5ff8f6d + d0d6179 commit 397b61e

File tree

10 files changed

+130
-4
lines changed

10 files changed

+130
-4
lines changed

documentation/config_docs.md

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ Refer [here](https://docs.github.com/en/free-pro-team@latest/developers/webhooks
2323
},
2424
"status_rules": {
2525
...
26+
},
27+
"project_owners": {
28+
...
2629
}
2730
}
2831
```
@@ -33,6 +36,7 @@ Refer [here](https://docs.github.com/en/free-pro-team@latest/developers/webhooks
3336
| `label_rules` | label rules config object | required field |
3437
| `prefix_rules` | prefix rules config object | required field |
3538
| `status_rules` | status rules config object | all status notifications are ignored |
39+
| `project_owners` | project owners config object | no project owners are defined |
3640

3741
## Label Options
3842

@@ -238,3 +242,37 @@ You can optionally provide a **status condition** to specify additional requirem
238242
}
239243
}
240244
```
245+
246+
## Project Owners
247+
248+
In GitHub, ["code owners"](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners) are the users/teams whose review is automatically requested when a PR modifying code in a given directory is opened. **Project owners** behave similarly, with two differences.
249+
250+
- Owners are defined _per PR label_, instead of _per directory_ (or directory pattern)
251+
- Definitions should be placed in the per-repo Monorobot configuration file, instead of a `CODEOWNERS` file
252+
253+
Draft PR behavior is similar to code owners. From GitHub documentation:
254+
255+
> Code owners are not automatically requested to review draft pull requests. [...] When you mark a draft pull request as ready for review, code owners are automatically notified.
256+
257+
The syntax for listing users is `username`. For teams, it is `org/team-name`.
258+
259+
Note that the owner of the personal access token cannot be a project owner, as GitHub disallows a user from self-requesting a review. Consider provisioning a separate bot user, or authenticating using a [GitHub App](https://docs.github.com/en/developers/apps/building-github-apps/authenticating-with-github-apps#accessing-api-endpoints-as-a-github-app) instead.
260+
261+
```json
262+
{
263+
...,
264+
"project_owners": {
265+
"rules": [
266+
{
267+
"label": "Label 1",
268+
"owners": ["user1", "user2", "org/team1"]
269+
},
270+
{
271+
"label": "Label 2",
272+
"owners": ["org/team2", "user3"]
273+
}
274+
]
275+
},
276+
...
277+
}
278+
```

lib/action.ml

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,27 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
208208
if config_was_modified then fetch_config () else Lwt.return @@ Ok ()
209209
| _ -> Lwt.return @@ Ok ()
210210

211+
let do_github_tasks ctx (req : Github.t) =
212+
let cfg = Context.get_config_exn ctx in
213+
let project_owners (pull_request : pull_request) repository number =
214+
match Github.get_project_owners pull_request cfg.project_owners with
215+
| Some reviewers ->
216+
( match%lwt Github_api.request_reviewers ~ctx ~repo:repository ~number ~reviewers with
217+
| Ok () -> Lwt.return_unit
218+
| Error e -> action_error e
219+
)
220+
| None -> Lwt.return_unit
221+
in
222+
match req with
223+
| Github.Pull_request
224+
{ action; pull_request = { draft = false; state = Open; _ } as pull_request; repository; number; _ } ->
225+
begin
226+
match action with
227+
| Ready_for_review | Labeled -> project_owners pull_request repository number
228+
| _ -> Lwt.return_unit
229+
end
230+
| _ -> Lwt.return_unit
231+
211232
let process_github_notification (ctx : Context.t) headers body =
212233
try%lwt
213234
let secrets = Context.get_secrets_exn ctx in
@@ -218,7 +239,7 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
218239
| Error e -> action_error e
219240
| Ok () ->
220241
let%lwt notifications = generate_notifications ctx payload in
221-
let%lwt () = send_notifications ctx notifications in
242+
let%lwt () = Lwt.join [ send_notifications ctx notifications; do_github_tasks ctx payload ] in
222243
( match ctx.state_filepath with
223244
| None -> Lwt.return_unit
224245
| Some path ->

lib/api.ml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,13 @@ module type Github = sig
1010
val get_pull_request : ctx:Context.t -> repo:repository -> number:int -> (pull_request, string) Result.t Lwt.t
1111

1212
val get_issue : ctx:Context.t -> repo:repository -> number:int -> (issue, string) Result.t Lwt.t
13+
14+
val request_reviewers
15+
: ctx:Context.t ->
16+
repo:repository ->
17+
number:int ->
18+
reviewers:request_reviewers_req ->
19+
(unit, string) Result.t Lwt.t
1320
end
1421

1522
module type Slack = sig

lib/api_local.ml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ module Github : Api.Github = struct
2222
let get_pull_request ~ctx:_ ~repo:_ ~number:_ = Lwt.return @@ Error "undefined for local setup"
2323

2424
let get_issue ~ctx:_ ~repo:_ ~number:_ = Lwt.return @@ Error "undefined for local setup"
25+
26+
let request_reviewers ~ctx:_ ~repo:_ ~number:_ ~reviewers:_ = Lwt.return @@ Error "undefined for local setup"
2527
end
2628

2729
module Slack_base : Api.Slack = struct

lib/api_remote.ml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,13 @@ module Github : Api.Github = struct
5151
| Ok res -> Lwt.return @@ Ok res
5252
| Error e -> Lwt.return @@ fmt_error "error while querying remote: %s\nfailed to get resource from %s" e url
5353

54+
let post_resource (ctx : Context.t) body url =
55+
let secrets = Context.get_secrets_exn ctx in
56+
let headers = build_headers ?token:secrets.gh_token () in
57+
match%lwt http_request ~headers ~body:(`Raw ("application/json; charset=utf-8", body)) `POST url with
58+
| Ok res -> Lwt.return @@ Ok res
59+
| Error e -> Lwt.return @@ fmt_error "POST to %s failed : %s" url e
60+
5461
let get_api_commit ~(ctx : Context.t) ~repo ~sha =
5562
let%lwt res = commits_url ~repo ~sha |> get_resource ctx in
5663
Lwt.return @@ Result.map res ~f:Github_j.api_commit_of_string
@@ -62,6 +69,11 @@ module Github : Api.Github = struct
6269
let get_issue ~(ctx : Context.t) ~repo ~number =
6370
let%lwt res = issues_url ~repo ~number |> get_resource ctx in
6471
Lwt.return @@ Result.map res ~f:Github_j.issue_of_string
72+
73+
let request_reviewers ~(ctx : Context.t) ~repo ~number ~reviewers =
74+
let body = Github_j.string_of_request_reviewers_req reviewers in
75+
let%lwt res = pulls_url ~repo ~number ^ "/requested_reviewers" |> post_resource ctx body in
76+
Lwt.return @@ Result.map res ~f:(fun _ -> ())
6577
end
6678

6779
module Slack : Api.Slack = struct

lib/config.atd

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
type status_rule <ocaml from="Rule"> = abstract
22
type prefix_rule <ocaml from="Rule"> = abstract
33
type label_rule <ocaml from="Rule"> = abstract
4+
type project_owners_rule <ocaml from="Rule"> = abstract
45

56
(* This type of rule is used for CI build notifications. *)
67
type status_rules = {
@@ -22,12 +23,18 @@ type label_rules = {
2223
rules: label_rule list;
2324
}
2425

26+
(* This type of rule is used for routing PR review requests to users. *)
27+
type project_owners = {
28+
rules: project_owners_rule list;
29+
}
30+
2531
(* This is the structure of the repository configuration file. It should be at the
2632
root of the monorepo, on the main branch. *)
2733
type config = {
2834
prefix_rules : prefix_rules;
2935
label_rules : label_rules;
3036
~status_rules <ocaml default="{allowed_pipelines = Some []; rules = []}"> : status_rules;
37+
~project_owners <ocaml default="{rules = []}"> : project_owners;
3138
?main_branch_name : string nullable; (* the name of the main branch; used to filter out notifications about merges of main branch into other branches *)
3239
}
3340

lib/github.atd

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,11 @@ type commit_comment_notification = {
269269
sender: github_user;
270270
}
271271

272+
type request_reviewers_req = {
273+
reviewers : string list;
274+
team_reviewers : string list;
275+
}
276+
272277
(* other generic events *)
273278
type event_notification = {
274279
repository: repository

lib/github.ml

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,9 @@ type gh_link =
9292
| Issue of repository * int
9393
| Commit of repository * commit_hash
9494

95-
let gh_re = Re2.create_exn {|^(.*)/(.+)/(.+)/(commit|pull|issues)/([a-z0-9]+)/?$|}
95+
let gh_link_re = Re2.create_exn {|^(.*)/(.+)/(.+)/(commit|pull|issues)/([a-z0-9]+)/?$|}
96+
97+
let gh_org_team_re = Re2.create_exn {|[a-zA-Z0-9\-]+/([a-zA-Z0-9\-]+)|}
9698

9799
(** [gh_link_of_string s] parses a URL string [s] to try to match a supported
98100
GitHub link type, generating repository endpoints if necessary *)
@@ -108,7 +110,7 @@ let gh_link_of_string url_str =
108110
match Uri.host url with
109111
| None -> None
110112
| Some host ->
111-
match Re2.find_submatches_exn gh_re path with
113+
match Re2.find_submatches_exn gh_link_re path with
112114
| [| _; prefix; Some owner; Some name; Some link_type; Some item |] ->
113115
let base = Option.value_map prefix ~default:host ~f:(fun p -> String.concat [ host; p ]) in
114116
let scheme = Uri.scheme url in
@@ -137,3 +139,19 @@ let gh_link_of_string url_str =
137139
with _ -> None
138140
end
139141
| _ | (exception Re2.Exceptions.Regex_match_failed _) -> None
142+
143+
let get_project_owners (pr : pull_request) ({ rules } : Config_t.project_owners) =
144+
List.fold_left pr.labels ~init:[] ~f:(fun acc l -> List.rev_append (Rule.Project_owners.match_rules l ~rules) acc)
145+
|> List.dedup_and_sort ~compare:String.compare
146+
|> List.partition_map ~f:(fun reviewer ->
147+
try
148+
let team = Re2.find_first_exn ~sub:(`Index 1) gh_org_team_re reviewer in
149+
Second team
150+
with Re2.Exceptions.Regex_match_failed _ -> First reviewer
151+
)
152+
|> fun (reviewers, team_reviewers) ->
153+
let already_requested = List.map ~f:(fun r -> r.login) pr.requested_reviewers in
154+
let already_requested_team = List.map ~f:(fun r -> r.slug) pr.requested_teams in
155+
let reviewers = List.filter ~f:(not $ List.mem already_requested ~equal:String.equal) reviewers in
156+
let team_reviewers = List.filter ~f:(not $ List.mem already_requested_team ~equal:String.equal) team_reviewers in
157+
if List.is_empty reviewers && List.is_empty team_reviewers then None else Some { reviewers; team_reviewers }

lib/rule.atd

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,4 +79,13 @@ type label_rule = {
7979
?allow <json name="match"> : string list nullable;
8080
?ignore : string list nullable;
8181
channel_name <json name="channel"> : string;
82-
}
82+
}
83+
84+
(* Requests reviews from [owners] if a PR is labeled with [label]. Owner format:
85+
- users: "username"
86+
- teams: "org/team"
87+
*)
88+
type project_owners_rule = {
89+
label: string;
90+
owners: string list;
91+
}

lib/rule.ml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,3 +125,10 @@ module Label = struct
125125
Stdio.printf " -> #%s\n%!" rule.channel_name
126126
)
127127
end
128+
129+
module Project_owners = struct
130+
let match_rules (l : Github_t.label) ~rules =
131+
match List.find rules ~f:(fun { label; _ } -> String.equal label l.name) with
132+
| Some { owners = []; _ } | None -> []
133+
| Some { owners; _ } -> owners
134+
end

0 commit comments

Comments
 (0)