Skip to content

Commit b0b0395

Browse files
author
Release Manager
committed
gh-40839: Fix in label synchronization bot according to issue #40758 <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes #12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes #12345". --> This PR fixes #40758. > The action https://github.com/sagemath/sage/blob/develop/.github/sync_labels.py should not change "needs work" (maybe "needs info" too, but we don't use that as much so I'm not sure if it has different behaviour currently) to "needs review" when the commit message matches the default message for a merge commit. Alternatives Considered This is achieved by ignoring merge commits when the latest commit date is determined in case a `s: needs_work` label exists. Since `s: needs_work` persists if there is a more recent review requesting changes this should solve the problem. > Merging develop should still trigger a new CI run. I don't imagine that this change would effect that though. This remains unchanged. > Check whether merging from the command line with git and merging from the GitHub web interface generate the same default merge commit messages. In both cases, the predefined message begins with `Merge` and contains the word `develop`. I just check that this is valid. However, note that the user (at least in the in the command line) has the option of changing the messages. Therefore, we generally cannot detect merge commits exactly. This PR also implements a new method `test_method` just for test purposes. This allows to run arbitrary methods of the class from the command line. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - #12345: short description why this is a dependency --> <!-- - #34567: ... --> URL: #40839 Reported by: Sebastian Oehms Reviewer(s): Sebastian Oehms, Vincent Macri
2 parents 3efb3f4 + 0a3cfe2 commit b0b0395

File tree

1 file changed

+48
-4
lines changed

1 file changed

+48
-4
lines changed

.github/sync_labels.py

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,17 @@ def get_commits(self):
408408
return self._commits
409409

410410
self._commits = self.view('commits')
411-
self._commit_date = max( com['committedDate'] for com in self._commits )
411+
412+
# ignore merge commits with the develop branch for _commit_date unless positive review is set
413+
date_commits = list(self._commits)
414+
if Status.positive_review.value not in self.get_labels():
415+
for com in self._commits:
416+
message = com['messageHeadline']
417+
if message.startswith('Merge') and 'develop' in message:
418+
debug('Ignore merge commit %s for commit_date' % com['oid'])
419+
date_commits.remove(com)
420+
421+
self._commit_date = max(com['committedDate'] for com in date_commits)
412422
info('Commits until %s for %s: %s' % (self._commit_date, self._issue, self._commits))
413423
return self._commits
414424

@@ -1034,6 +1044,35 @@ def run_tests(self):
10341044
elif self.is_pull_request():
10351045
self.run(action)
10361046

1047+
def test_method(self, method, *args, **kwds):
1048+
r"""
1049+
Run the given method for testing.
1050+
1051+
EXAMPLES::
1052+
1053+
sage$ python .github/sync_labels.py https://github.com/sagemath/sage/pull/40634 soehms is_auth_team_member "{'login': 'soehms'}" -t
1054+
INFO:root:cmdline_args (4) ['https://github.com/sagemath/sage/pull/40634', 'soehms', 'is_auth_team_member', "{'login': 'soehms'}"]
1055+
...
1056+
DEBUG:root:call is_auth_team_member with args () and kwds {'login': 'soehms'}
1057+
DEBUG:root:================================================================================
1058+
DEBUG:root:Execute command: gh api -X GET -H "Accept: application/vnd.github+json" /orgs/sagemath/teams/triage/memberships/soehms
1059+
INFO:root:User soehms is a member of triage
1060+
INFO:root:result of is_auth_team_member with args () and kwds {'login': 'soehms'} is True
1061+
INFO:root:================================================================================
1062+
...
1063+
"""
1064+
if hasattr(self, method):
1065+
meth = self.__getattribute__(method)
1066+
if callable(meth):
1067+
debug('call %s with args %s and kwds %s' % (method, args, kwds))
1068+
debug('='*80)
1069+
res = meth(*args, **kwds)
1070+
info('result of %s with args %s and kwds %s is %s' % (method, args, kwds, res))
1071+
info('='*80)
1072+
debug('state of self: %s' % self.__dict__)
1073+
return
1074+
raise ValueError('%s is not a method of %s' % (method, self))
1075+
10371076

10381077
###############################################################################
10391078
# Main
@@ -1068,8 +1107,10 @@ def run_tests(self):
10681107
num_args = len(cmdline_args)
10691108
info('cmdline_args (%s) %s' % (num_args, cmdline_args))
10701109

1071-
if run_tests and num_args in (1,2):
1072-
if num_args == 2:
1110+
if run_tests:
1111+
if num_args == 4:
1112+
url, actor, method, args = cmdline_args
1113+
elif num_args == 2:
10731114
url, actor = cmdline_args
10741115
else:
10751116
url, = cmdline_args
@@ -1079,7 +1120,10 @@ def run_tests(self):
10791120
info('actor: %s' % actor)
10801121

10811122
gh = GhLabelSynchronizer(url, actor)
1082-
gh.run_tests()
1123+
if num_args == 4:
1124+
gh.test_method(method, **eval(args))
1125+
else:
1126+
gh.run_tests()
10831127

10841128
elif num_args == 5:
10851129
action, url, actor, label, rev_state = cmdline_args

0 commit comments

Comments
 (0)