Skip to content

Commit abd3663

Browse files
committed
Merge branch 'yasu/branch-filter'
* yasu/branch-filter: rule: use keyword "any" for global filter override docs: s/if absent/default/ test: use filter_main_branch in test config rule: use main_branch_name for global filtering docs: clean up prefix rule fields table docs: info about branch filter tests: add cases for local+default branch filter rule: distinguish undeclared vs empty local filter rule: add global `default_branch_filters` rule: add `branch_filters` prefix rule
2 parents 39cafee + 221b327 commit abd3663

13 files changed

+925
-19
lines changed

documentation/config_docs.md

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ Refer [here](https://docs.github.com/en/free-pro-team@latest/developers/webhooks
2929

3030
| value | description | optional | default |
3131
|-|-|-|-|
32-
| `main_branch_name` | main branch used for the repo; filtering notifications about merges of main into other branches | Yes | - |
32+
| `main_branch_name` | main branch used for the repo; filtering notifications about merges of main into other branches, and constraining prefix rule application | Yes | - |
3333
| `label_rules` | label rules config object | No | - |
3434
| `prefix_rules` | prefix rules config object | No | - |
3535
| `status_rules` | status rules config object | No | - |
@@ -97,6 +97,7 @@ A **label rule** specifies whether or not a Slack channel should be notified, ba
9797
```json
9898
"prefix_rules": {
9999
"default_channel": "default",
100+
"filter_main_branch": true,
100101
"rules": [
101102
{
102103
"match": [
@@ -109,6 +110,7 @@ A **label rule** specifies whether or not a Slack channel should be notified, ba
109110
"backend/megaindex",
110111
"backend/ahrefskit"
111112
],
113+
"branch_filters": "any",
112114
"channel": "backend"
113115
},
114116
{
@@ -117,16 +119,27 @@ A **label rule** specifies whether or not a Slack channel should be notified, ba
117119
]
118120
},
119121
```
122+
| value | description | default |
123+
|-|-|-|
124+
| `default_channel` | same behavior as label rule `default_channel` | |
125+
| `filter_main_branch` | if true and `main_branch_name` is declared, use main branch to filter rules that have no local filter; otherwise, don't apply branch filtering and show `distinct` commits only | false |
126+
| `rules` | list of `prefix_rule` objects | required field |
120127

121128
### Prefix Rule
122129

123130
A **prefix rule** specifies whether or not a Slack channel should be notified, based on the filenames present in the commits associated with the given payload. The semantics for the `match` and `ignore` fields are the same as those for label rules (see above).
124131

125-
| value | description | optional | default |
126-
|-|-|-|-|
127-
| `match` | if commit files have any prefix in this list, they should be routed to the channel | Yes | all prefixes matched if no list provided |
128-
| `ignore` | if commit files have any prefix in this list, they shouldn't be routed to the channel (even if they have any `match` prefixes) | Yes | - |
129-
| `channel` | channel to notify if the rule is matched | No | - |
132+
Default behavior is to apply each rule regardless of what branch is pushed, and when a rule is matched, show its `distinct` commits only.
133+
Branch filters limit rule application to selected branches, and shows _all_ commits on match.
134+
The filters can be declared globally with `filter_main_branch` (see above), or locally per rule with `branch_filters`, where the latter takes precedence.
135+
To ignore a globally declared filter for a single rule, declare one locally with value "any", as shown in the example above.
136+
137+
| value | description | default |
138+
|-|-|-|
139+
| `match` | if commit files have any prefix in this list, they should be routed to the channel | all prefixes matched |
140+
| `ignore` | if commit files have any prefix in this list, they shouldn't be routed to the channel (even if they have any `match` prefixes) | fall back on `match` field behavior |
141+
| `branch_filters` | consider commits only if pushed ref branch is in this list; set to "any" to ignore `filter_main_branch` for this rule | fall back on `filter_main_branch` field behavior (see above) |
142+
| `channel` | channel to notify if the rule is matched | required field |
130143

131144
## Status Options
132145

lib/action.ml

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,20 +15,24 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
1515
let partition_push (cfg : Config_t.config) n =
1616
let default = Option.to_list cfg.prefix_rules.default_channel in
1717
let rules = cfg.prefix_rules.rules in
18+
let branch = Github.commits_branch_of_ref n.ref in
19+
let main_branch = if cfg.prefix_rules.filter_main_branch then cfg.main_branch_name else None in
20+
let filter_by_branch = Rule.Prefix.filter_by_branch ~branch ~main_branch in
1821
n.commits
19-
|> List.filter ~f:(fun c -> c.distinct)
2022
|> List.filter ~f:(fun c ->
21-
let branch = Github.commits_branch_of_ref n.ref in
2223
let skip = Github.is_main_merge_message ~msg:c.message ~branch cfg in
2324
if skip then log#info "main branch merge, ignoring %s: %s" c.id (first_line c.message);
2425
not skip)
2526
|> List.concat_map ~f:(fun commit ->
27+
let rules = List.filter ~f:(filter_by_branch ~distinct:commit.distinct) rules in
2628
let matched_channel_names =
2729
Github.modified_files_of_commit commit
2830
|> List.filter_map ~f:(Rule.Prefix.match_rules ~rules)
2931
|> List.dedup_and_sort ~compare:String.compare
3032
in
31-
let channel_names = if List.is_empty matched_channel_names then default else matched_channel_names in
33+
let channel_names =
34+
if List.is_empty matched_channel_names && commit.distinct then default else matched_channel_names
35+
in
3236
List.map channel_names ~f:(fun n -> n, commit))
3337
|> Map.of_alist_multi (module String)
3438
|> Map.map ~f:(fun commits -> { n with commits })

lib/atd_adapters.ml

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
module List_or_default_field = struct
2+
open Atdgen_runtime.Json_adapter
3+
4+
module type Param = sig
5+
(** the name of the field *)
6+
val field_name : string
7+
8+
(** string alias a user can use to denote default value *)
9+
val default_alias : string
10+
11+
(** Yojson representation of default value *)
12+
val default_value : Yojson.Safe.t
13+
end
14+
15+
module Make (P : Param) : S = struct
16+
open P
17+
18+
let assoc_replace l k v = List.map (fun x -> if fst x = k then k, v else x) l
19+
20+
let normalize x =
21+
match x with
22+
| `Assoc fields ->
23+
begin
24+
match List.assoc field_name fields with
25+
| `String s when String.equal s default_alias -> `Assoc (assoc_replace fields field_name default_value)
26+
| _ | (exception Not_found) -> x
27+
end
28+
| malformed -> malformed
29+
30+
let restore x =
31+
match x with
32+
| `Assoc fields ->
33+
begin
34+
match List.assoc field_name fields with
35+
| value when Yojson.Safe.equal value default_value -> `Assoc (assoc_replace fields field_name (`String "any"))
36+
| _ | (exception Not_found) -> x
37+
end
38+
| malformed -> malformed
39+
end
40+
end
41+
42+
module Branch_filters_adapter = List_or_default_field.Make (struct
43+
let field_name = "branch_filters"
44+
45+
let default_alias = "any"
46+
47+
let default_value = `List []
48+
end)

lib/config.atd

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ type status_rules = {
1212
files they are related to. *)
1313
type prefix_rules = {
1414
?default_channel: string nullable; (* if none of the rules is matching *)
15+
~filter_main_branch <ocaml default="false">: bool;
1516
rules: prefix_rule list;
1617
}
1718

lib/rule.atd

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,9 @@ type status_rule = {
6666
type prefix_rule = {
6767
?allow <json name="match"> : string list nullable;
6868
?ignore : string list nullable;
69+
?branch_filters : string list nullable;
6970
channel_name <json name="channel"> : string;
70-
}
71+
} <json adapter.ocaml="Atd_adapters.Branch_filters_adapter">
7172

7273
(* A payload matches a label rule with a channel name if absent from the ignore list
7374
and present in the allow list. Both `allow` and `ignore` are optional. If `allow`

lib/rule.ml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,18 @@ module Status = struct
3939
end
4040

4141
module Prefix = struct
42+
(** Filters prefix rules based on branch filtering config and current commit.
43+
Prioritizes local filters over main branch one. Only allows distinct commits
44+
if no filter is matched. *)
45+
let filter_by_branch ~branch ~main_branch ~distinct rule =
46+
match rule.branch_filters with
47+
| Some (_ :: _ as filters) -> List.mem filters branch ~equal:String.equal
48+
| Some [] -> distinct
49+
| None ->
50+
match main_branch with
51+
| Some main_branch -> String.equal main_branch branch
52+
| None -> distinct
53+
4254
(** `match_rules f rs` returns the channel name of a rule in `rs` that matches
4355
file name `f` with the longest prefix, if one exists. A rule `r` matches
4456
`f` with prefix length `l`, if `f` has no prefix in `r.ignore` and `l` is

0 commit comments

Comments
 (0)