ci: various set_assignee fixes
Cleanup of script and change how we assign PR in general:
Make sure that when multiple areas are changed, including platform or
driver changes, we set assignee of the general areas and not the
platform maintainers. For example, if I change 100 SoC files and touch
the kernel file, the assignee is set to the kernel maintainer.
First, we should as much possible try and split such changes and not
have common area changes with platform changes at the same time. Second,
it is important that such changes which affect everyone are reviewed and
approved by the maintainer of the general areas rather than leave them
to the platform maintainer.
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
diff --git a/scripts/set_assignees.py b/scripts/set_assignees.py
index fb1987c..aadfde1 100755
--- a/scripts/set_assignees.py
+++ b/scripts/set_assignees.py
@@ -64,88 +64,92 @@
labels = set()
area_counter = defaultdict(int)
- maint = defaultdict(int)
+ found_maintainers = defaultdict(int)
num_files = 0
all_areas = set()
fn = list(pr.get_files())
+
+ # one liner PRs should be trivial
+ if pr.commits == 1 and (pr.additions <= 1 and pr.deletions <= 1):
+ labels = {'trivial'}
+
if len(fn) > 500:
log(f"Too many files changed ({len(fn)}), skipping....")
return
- for f in pr.get_files():
+
+ for changed_file in fn:
num_files += 1
- log(f"file: {f.filename}")
- areas = maintainer_file.path2areas(f.filename)
+ log(f"file: {changed_file.filename}")
+ areas = maintainer_file.path2areas(changed_file.filename)
- if areas:
- all_areas.update(areas)
- for a in areas:
- area_counter[a.name] += 1
- labels.update(a.labels)
- for p in a.maintainers:
- maint[p] += 1
+ if not areas:
+ continue
- ac = dict(sorted(area_counter.items(), key=lambda item: item[1], reverse=True))
- log(f"Area matches: {ac}")
+ all_areas.update(areas)
+ is_instance = False
+ sorted_areas = sorted(areas, key=lambda x: 'Platform' in x.name, reverse=True)
+ for area in sorted_areas:
+ c = 1 if not is_instance else 0
+
+ area_counter[area] += c
+ labels.update(area.labels)
+ # FIXME: Here we count the same file multiple times if it exists in
+ # multiple areas with same maintainer
+ for area_maintainer in area.maintainers:
+ found_maintainers[area_maintainer] += c
+
+ if 'Platform' in area.name:
+ is_instance = True
+
+ area_counter = dict(sorted(area_counter.items(), key=lambda item: item[1], reverse=True))
+ log(f"Area matches: {area_counter}")
log(f"labels: {labels}")
# Create a list of collaborators ordered by the area match
collab = list()
- for a in ac:
- collab += maintainer_file.areas[a].maintainers
- collab += maintainer_file.areas[a].collaborators
+ for area in area_counter:
+ collab += maintainer_file.areas[area.name].maintainers
+ collab += maintainer_file.areas[area.name].collaborators
collab = list(dict.fromkeys(collab))
log(f"collab: {collab}")
- sm = dict(sorted(maint.items(), key=lambda item: item[1], reverse=True))
+ _all_maintainers = dict(sorted(found_maintainers.items(), key=lambda item: item[1], reverse=True))
log(f"Submitted by: {pr.user.login}")
- log(f"candidate maintainers: {sm}")
+ log(f"candidate maintainers: {_all_maintainers}")
- maintainer = "None"
- maintainers = list(sm.keys())
+ maintainers = list(_all_maintainers.keys())
+ assignee = None
- prop = 0
- if maintainers:
- maintainer = maintainers[0]
+ # we start with areas with most files changed and pick the maintainer from the first one.
+ # if the first area is an implementation, i.e. driver or platform, we
+ # continue searching for any other areas
+ for area, count in area_counter.items():
+ if count == 0:
+ continue
+ if len(area.maintainers) > 0:
+ assignee = area.maintainers[0]
- if len(ac) > 1 and list(ac.values())[0] == list(ac.values())[1]:
- for aa in ac:
- if 'Documentation' in aa:
- log("++ With multiple areas of same weight including docs, take something else other than Documentation as the maintainer")
- for a in all_areas:
- if (a.name == aa and
- a.maintainers and a.maintainers[0] == maintainer and
- len(maintainers) > 1):
- maintainer = maintainers[1]
- elif 'Platform' in aa:
- log("++ Platform takes precedence over subsystem...")
- log(f"Set maintainer of area {aa}")
- for a in all_areas:
- if a.name == aa:
- if a.maintainers:
- maintainer = a.maintainers[0]
- break
+ if 'Platform' not in area.name:
+ break
+ # if the submitter is the same as the maintainer, check if we have
+ # multiple maintainers
+ if len(maintainers) > 1 and pr.user.login == assignee:
+ log("Submitter is same as Assignee, trying to find another assignee...")
+ aff = list(area_counter.keys())[0]
+ for area in all_areas:
+ if area.name == aff:
+ if len(area.maintainers) > 1:
+ assignee = area.maintainers[1]
+ else:
+ log(f"This area has only one maintainer, keeping assignee as {assignee}")
- # if the submitter is the same as the maintainer, check if we have
- # multiple maintainers
- if pr.user.login == maintainer:
- log("Submitter is same as Assignee, trying to find another assignee...")
- aff = list(ac.keys())[0]
- for a in all_areas:
- if a.name == aff:
- if len(a.maintainers) > 1:
- maintainer = a.maintainers[1]
- else:
- log(f"This area has only one maintainer, keeping assignee as {maintainer}")
-
- prop = (maint[maintainer] / num_files) * 100
- if prop < 20:
- maintainer = "None"
-
- log(f"Picked maintainer: {maintainer} ({prop:.2f}% ownership)")
- log("+++++++++++++++++++++++++")
+ if assignee:
+ prop = (found_maintainers[assignee] / num_files) * 100
+ log(f"Picked assignee: {assignee} ({prop:.2f}% ownership)")
+ log("+++++++++++++++++++++++++")
# Set labels
if labels:
@@ -197,9 +201,9 @@
ms = []
# assignees
- if maintainer != 'None' and not pr.assignee:
+ if assignee and not pr.assignee:
try:
- u = gh.get_user(maintainer)
+ u = gh.get_user(assignee)
ms.append(u)
except GithubException:
log(f"Error: Unknown user")