Skip to content

Commit 23e4848

Browse files
Vojtech HorkyAndrija Kolic
authored andcommitted
[GR-69970] Code owners: do not suggest author as reviewer.
PullRequest: mx/1970
2 parents bb855b5 + 2b0dc39 commit 23e4848

File tree

3 files changed

+93
-10
lines changed

3 files changed

+93
-10
lines changed

src/mx/_impl/mx_codeowners.py

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -232,24 +232,33 @@ def get_owners_of(self, filepath):
232232
mx.logv(f"File {filepath} owned by {result} (looked into {owners_files})")
233233
return result
234234

235-
def _summarize_owners(all_owners):
235+
def _remove_author_from_owner_list(owners, author):
236+
without_author = []
237+
for owner in owners:
238+
if not author or owner != author:
239+
without_author.append(owner)
240+
return without_author
241+
242+
def _summarize_owners(all_owners, author):
236243
must_review = set()
237244
anyof_reviewers = []
238245
mandatory_approvals = []
239246

240247
for owners in all_owners:
241-
for owner in owners.get('all', []):
242-
must_review.add(owner)
248+
must_review.update(_remove_author_from_owner_list(owners.get('all', []), author))
243249

244250
for owners in all_owners:
245-
if owners.get('any', []):
251+
any_owners = owners.get('any', [])
252+
if any_owners:
246253
# One reviewer is already present? Skip this completely
247-
if not _is_some_item_in_set(owners['any'], must_review):
248-
anyof_reviewers.append(owners['any'])
254+
if _is_some_item_in_set(any_owners, must_review):
255+
continue
256+
anyof_reviewers.append(_remove_author_from_owner_list(any_owners, author))
249257

250258
for owners in all_owners:
251-
if owners.get('at_least_one_mandatory_approver'):
252-
mandatory_approvals.append(owners['at_least_one_mandatory_approver'])
259+
mandatory_approvers = owners.get('at_least_one_mandatory_approver', [])
260+
if mandatory_approvers:
261+
mandatory_approvals.append(_remove_author_from_owner_list(mandatory_approvers, author))
253262

254263
return {
255264
"all": sorted(must_review),
@@ -419,7 +428,7 @@ def codeowners(args):
419428
file_owners = {f: owners.get_owners_of(os.path.abspath(f)) for f in args.files}
420429
repro_data['owners'] = file_owners
421430

422-
reviewers = _summarize_owners(file_owners.values())
431+
reviewers = _summarize_owners(file_owners.values(), args.author)
423432

424433
if reviewers['at_least_one_mandatory_approver']:
425434
approvers_yet_to_approve = []

src/mx/mx_version.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
# The version must be updated for every PR (checked in CI) and the comment should reflect the PR's issue
2-
version = "7.67.0" # GR-70656 Introduce BenchmarkDispatcher and rework BenchmarkExecutionContext
2+
version = "7.67.1" # GR-69970 Code owners: do not suggest author as reviewer

tests/code_owners_tests.py

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,80 @@ def test_codeowners_json_output_generate_cases():
626626
},
627627
)
628628

629+
yield (
630+
"do not suggest author as reviewer",
631+
{
632+
"OWNERS.toml": """
633+
[[rule]]
634+
files = "*.md"
635+
636+
[[rule]]
637+
files = "*.txt"
638+
639+
""",
640+
},
641+
["README.md", "README.txt"],
642+
{
643+
"branch": None,
644+
"files": ["README.md", "README.txt"],
645+
"mx_version": str(mx.version),
646+
"owners": {
647+
"README.md": {"all": ["[email protected]", "[email protected]"], "trace": ["OWNERS.toml"]},
648+
"README.txt": {"any": ["[email protected]", "[email protected]"], "trace": ["OWNERS.toml"]},
649+
},
650+
"pull_request": {
651+
"approvals": ["[email protected]"],
652+
"author": "[email protected]",
653+
654+
"suggestion": {
655+
"acquire_approval": [],
656+
657+
"details": {
658+
"all": ["[email protected]"],
659+
"any": ["[email protected]"],
660+
"at_least_one_mandatory_approver": [],
661+
},
662+
},
663+
},
664+
"version": 1,
665+
},
666+
)
667+
668+
yield (
669+
"do not suggest author as reviewer: author is only owner",
670+
{
671+
"OWNERS.toml": """
672+
[[rule]]
673+
files = "*.md"
674+
675+
[[rule]]
676+
files = "*.txt"
677+
678+
""",
679+
},
680+
["README.md", "README.txt"],
681+
{
682+
"branch": None,
683+
"files": ["README.md", "README.txt"],
684+
"mx_version": str(mx.version),
685+
"owners": {
686+
"README.md": {"all": ["[email protected]"], "trace": ["OWNERS.toml"]},
687+
"README.txt": {"any": ["[email protected]"], "trace": ["OWNERS.toml"]},
688+
},
689+
"pull_request": {
690+
"approvals": ["[email protected]"],
691+
"author": "[email protected]",
692+
693+
"suggestion": {
694+
"acquire_approval": [],
695+
"add": [],
696+
"details": {"all": [], "any": [], "at_least_one_mandatory_approver": []},
697+
},
698+
},
699+
"version": 1,
700+
},
701+
)
702+
629703
yield (
630704
"multiple files",
631705
{

0 commit comments

Comments
 (0)