From 3fb02406d40b58d055ebdae16b21e59ce8a9464f Mon Sep 17 00:00:00 2001 From: Sebastian Date: Wed, 1 Feb 2023 23:42:17 +0100 Subject: [PATCH 1/6] sync_labels initial --- .github/sync_labels.py | 253 ++++++++++++++++++++++++++++++ .github/workflows/sync_labels.yml | 40 +++++ 2 files changed, 293 insertions(+) create mode 100755 .github/sync_labels.py create mode 100644 .github/workflows/sync_labels.yml diff --git a/.github/sync_labels.py b/.github/sync_labels.py new file mode 100755 index 0000000000..d0ebe1385e --- /dev/null +++ b/.github/sync_labels.py @@ -0,0 +1,253 @@ +#!/usr/bin/python +# -*- coding: utf-8 -*- + +r""" +Python script to sync labels that are migrated from Trac selection lists. +""" + +############################################################################## +# Copyright (C) 2023 Sebastian Oehms +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 2 of the License, or +# (at your option) any later version. +# http://www.gnu.org/licenses/ +############################################################################## + +import os +import sys +import json +from enum import Enum + +class SelectionList(Enum): + """ + Abstract Enum for selection lists. + """ + def succ(self): + """ + Return the successor of `self`. + """ + l = list(self.__class__) + i = l.index(self) + if i + 1 == len(l): + if i: + return l[i-1] + else: + return None + return l[i+1] + +class Priority(SelectionList): + """ + Enum for priority lables. + """ + blocker = 'p: blocker /1' + critical = 'p: critical /2' + major = 'p: major /3' + minor = 'p: minor /4' + trivial = 'p: trivial /5' + +class State(SelectionList): + """ + Enum for state lables. + """ + closed = 'closed' + positive_review = 'positive review' + needs_review = 'needs review' + needs_work = 'needs work' + needs_info = 'needs info' + new = 'new' + +class IssueType(SelectionList): + """ + Enum for type lables. + """ + bug = 'bug' + enhancement = 'enhancement' + task = 'task' + +def selection_list(label): + """ + Return the selection list to which `label` belongs to. + """ + for sel_list in [Priority, State, IssueType]: + for item in sel_list: + if label == item.value: + return sel_list + return None + +class GhLabelSynchronizer: + """ + Handler for access to GitHub issue via the `gh` in the bash command line + of the GitHub runner. + """ + def __init__(self, url, labels): + """ + Python constructor sets the issue / PR url and list of active labels. + """ + self._url = url + self._labels = labels + number = os.path.basename(url) + self._pr = True + self._issue = 'pull request #%s' % number + if url.rfind('issue') != -1: + self._issue = 'isuue #%s' % number + self._pr = False + + def active_partners(self, label): + """ + Return the list of other labels from the selection list + of the given one that are already present on the issue / PR. + """ + sel_list = selection_list(label) + val = [i.value for i in sel_list] + return [l for l in self._labels if l in val and not l == label] + + def edit(self, arg, option): + """ + Perform a system call to `gh` to edit an issue or PR. + """ + issue = 'issue' + if self._pr: + issue = 'pr' + cmd = 'gh %s edit %s %s "%s"' % (issue, self._url, option, arg) + os.system(cmd) + + def add_comment(self, text): + """ + Perform a system call to `gh` to add a comment to an issue or PR. + """ + issue = 'issue' + if self._pr: + issue = 'pr' + cmd = 'gh %s comment %s -b "%s"' % (issue, self._url, text) + os.system(cmd) + print('Comment %s added to %s' % (text, self._issue)) + + def add_label(self, label): + """ + Add the given label to the issue or PR. + """ + if not label in self._labels: + self.edit(label, '--add-label') + print('Label %s added to %s' % (label, self._issue)) + + def add_default_label(self, label): + """ + Add the given label if there is no active partner. + """ + if not self.active_partners(label): + self.add_label(label) + + def on_label_add(self, label): + """ + Check if the given label belongs to a selection list. + If so, remove all other labels of that list. + """ + sel_list = selection_list(label) + if not sel_list: + return + item = sel_list(label) + if sel_list is State and self._pr: + if item not in [State.needs_info, State.needs_review]: + self.add_comment('Label can not be added. Please use the corresponding functionality of GitHub') + self.remove_label(label) + return + + for other in sel_list: + if other != item: + self.remove_label(other.value) + + def select_label(self, label): + """ + Add the given label and remove all others. + """ + self.add_label(label) + self.on_label_add(label) + + def remove_label(self, label): + """ + Remove the given label from the issue or PR of the handler. + """ + if label in self._labels: + self.edit(label, '--remove-label') + print('Label %s removed from %s' % (label, self._issue)) + + def on_label_remove(self, label): + """ + Check if the given label belongs to a selection list. If so, reject + the removement in case it represents the stat of a PR. In all other + cases add the successor of the label except if there is none or there + exists another label of the corresponding list. + """ + sel_list = selection_list(label) + if not sel_list: + return + item = sel_list(label) + if sel_list is State and self._pr: + if item in [State.positive_review, State.needs_work, State.close]: + self.add_comment('Label can not be removed. Please use the corresponding functionality of GitHub') + self.select_label(label) + return + + if not self.active_partners(label): + succ = sel_list(label).succ() + if succ: + # add the next weaker label + self.select_label(succ.value) + else: + self.add_comment('Label can not be removed since there is no replacement') + self.select_label(label) + + +############################################################################### +# Main +############################################################################### +cmdline_args = sys.argv[1:] +print('cmdline_args', len(cmdline_args), cmdline_args) +if len(cmdline_args) < 6: + print('Need 6 arguments: action, url, label, state, issue_labels, pr_labels' ) + exit +else: + action, url, label, state, issue_labels, pr_labels = cmdline_args + +labels = json.loads(issue_labels) +if not labels: + labels = json.loads(pr_labels) + +base_url = url +for i in range(3): base_url = os.path.dirname(base_url) +print("action", action) +print("url", url) +print("label", label) +print("state", state) +print("labels", labels) +print("base_url", base_url) + +gh = GhLabelSynchronizer(url, labels) + + +if action == 'opened': + gh.add_default_label(Priority.major.value) + gh.add_default_label(State.new.value) + gh.add_default_label(IssueType.task.value) + +if action == 'labeled': + gh.on_label_add(label) + +if action == 'unlabeled': + gh.on_label_remove(label) + +if action == 'submitted': + if state == 'approved': + gh.select_label(State.positive_review.value) + + if state == 'changes_requested': + gh.select_label(State.needs_work.value) + + if state == 'commented': + # just for testing + gh.select_label(State.needs_work.value) + +if action in ('review_requested', 'ready_for_review'): + gh.select_label(State.needs_review.value) diff --git a/.github/workflows/sync_labels.yml b/.github/workflows/sync_labels.yml new file mode 100644 index 0000000000..1db5e88fd0 --- /dev/null +++ b/.github/workflows/sync_labels.yml @@ -0,0 +1,40 @@ +# This workflow synchronizels groups of labels that correspond +# to items of selection list in Trac. It controls that in each +# such case there is just one label of the list present. +# Furthermore in the case of the state it checks the labels +# to coincide with the corresponding review state. + +name: Synchronize selection list lables + +on: + pull_request_review: + types: [submitted] + issues: + types: [opened, labeled, unlabeled] + pull_request: + types: [opened, labeled, unlabeled] + +jobs: + synchronize: + runs-on: ubuntu-latest + steps: + # Download the Python script + - name: Download script + id: download + run: | + curl -L -O "https://raw.githubusercontent.com/soehms/trac-to-github/master/.github/sync_labels.py" + chmod a+x sync_labels.py + + # Perform synchronization + - name: Call script + run: ./sync_labels.py $ACTION $ISSUE_URL $PR_URL "$LABEL" "$STATE" "$ISSUE_LABELS" "$PR_LABELS" + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + ACTION: ${{ github.event.action }} + PR_URL: ${{ github.event.pull_request.html_url }} + ISSUE_URL: ${{ github.event.issue.html_url }} + STATE: ${{ github.event.review.state }} + LABEL: ${{ github.event.label.name }} + ISSUE_LABELS: ${{ toJson(github.event.issue.labels.*.name) }} + PR_LABELS: ${{ toJson(github.event.pull_request.labels.*.name) }} + From e4f6d3b58be8ac7cbc5dc2e45df9383ee28f8dc7 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Thu, 2 Feb 2023 08:14:47 +0100 Subject: [PATCH 2/6] remove unused labels --- .github/sync_labels.py | 3 --- .github/workflows/sync_labels.yml | 4 +++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/sync_labels.py b/.github/sync_labels.py index d0ebe1385e..4c377626f4 100755 --- a/.github/sync_labels.py +++ b/.github/sync_labels.py @@ -51,12 +51,10 @@ class State(SelectionList): """ Enum for state lables. """ - closed = 'closed' positive_review = 'positive review' needs_review = 'needs review' needs_work = 'needs work' needs_info = 'needs info' - new = 'new' class IssueType(SelectionList): """ @@ -64,7 +62,6 @@ class IssueType(SelectionList): """ bug = 'bug' enhancement = 'enhancement' - task = 'task' def selection_list(label): """ diff --git a/.github/workflows/sync_labels.yml b/.github/workflows/sync_labels.yml index 1db5e88fd0..bdfd85cfd7 100644 --- a/.github/workflows/sync_labels.yml +++ b/.github/workflows/sync_labels.yml @@ -22,8 +22,10 @@ jobs: - name: Download script id: download run: | - curl -L -O "https://raw.githubusercontent.com/soehms/trac-to-github/master/.github/sync_labels.py" + curl -L -O "https://raw.githubusercontent.com/"$REPO"/master/.github/sync_labels.py" chmod a+x sync_labels.py + env: + REPO: ${{ github.repository }} # Perform synchronization - name: Call script From c48248039b691493eb23eb8076c7bdaa41ba2eb6 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Thu, 2 Feb 2023 08:37:47 +0100 Subject: [PATCH 3/6] adapt the defaults --- .github/sync_labels.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/sync_labels.py b/.github/sync_labels.py index 4c377626f4..47c3a303ce 100755 --- a/.github/sync_labels.py +++ b/.github/sync_labels.py @@ -226,8 +226,8 @@ def on_label_remove(self, label): if action == 'opened': gh.add_default_label(Priority.major.value) - gh.add_default_label(State.new.value) - gh.add_default_label(IssueType.task.value) + gh.add_default_label(IssueType.enhancement.value) + gh.add_default_label(State.needs_review.value) if action == 'labeled': gh.on_label_add(label) From a0e07539eb3529129963c51252f582823e306925 Mon Sep 17 00:00:00 2001 From: Sebastian Oehms Date: Fri, 3 Feb 2023 18:36:57 +0100 Subject: [PATCH 4/6] use label list via gh view --- .github/sync_labels.py | 130 +++++++++++++++++++++--------- .github/workflows/sync_labels.yml | 9 +-- 2 files changed, 94 insertions(+), 45 deletions(-) diff --git a/.github/sync_labels.py b/.github/sync_labels.py index 47c3a303ce..070ef34ff2 100755 --- a/.github/sync_labels.py +++ b/.github/sync_labels.py @@ -17,7 +17,8 @@ import os import sys -import json +from logging import info, warning, getLogger, INFO +from json import loads from enum import Enum class SelectionList(Enum): @@ -78,31 +79,52 @@ class GhLabelSynchronizer: Handler for access to GitHub issue via the `gh` in the bash command line of the GitHub runner. """ - def __init__(self, url, labels): + def __init__(self, url): """ Python constructor sets the issue / PR url and list of active labels. """ self._url = url - self._labels = labels + self._labels = None number = os.path.basename(url) self._pr = True self._issue = 'pull request #%s' % number if url.rfind('issue') != -1: - self._issue = 'isuue #%s' % number + self._issue = 'issue #%s' % number self._pr = False + info('Create label handler for %s' % self._issue) - def active_partners(self, label): + + def is_pull_request(self): """ - Return the list of other labels from the selection list - of the given one that are already present on the issue / PR. + Return if we are treating a pull request. """ - sel_list = selection_list(label) - val = [i.value for i in sel_list] - return [l for l in self._labels if l in val and not l == label] + return self._pr + + def view(self, key): + """ + Return data obtained from `gh` command `view`. + """ + issue = 'issue' + if self._pr: + issue = 'pr' + cmd = 'gh %s view %s --json %s' % (issue, self._url, key) + from subprocess import check_output + return loads(check_output(cmd, shell=True)) + + def get_labels(self): + """ + Return the list of labels of the issue resp. PR. + """ + if self._labels: + return self._labels + data = self.view('labels')['labels'] + self._labels = [l['name'] for l in data] + info('List of labels for %s: %s' % (self._issue, self._labels)) + return self._labels def edit(self, arg, option): """ - Perform a system call to `gh` to edit an issue or PR. + Perform a system call to `gh` to edit an resp. PR. """ issue = 'issue' if self._pr: @@ -110,6 +132,15 @@ def edit(self, arg, option): cmd = 'gh %s edit %s %s "%s"' % (issue, self._url, option, arg) os.system(cmd) + def active_partners(self, label): + """ + Return the list of other labels from the selection list + of the given one that are already present on the issue / PR. + """ + sel_list = selection_list(label) + val = [i.value for i in sel_list] + return [l for l in self.get_labels() if l in val and not l == label] + def add_comment(self, text): """ Perform a system call to `gh` to add a comment to an issue or PR. @@ -119,15 +150,15 @@ def add_comment(self, text): issue = 'pr' cmd = 'gh %s comment %s -b "%s"' % (issue, self._url, text) os.system(cmd) - print('Comment %s added to %s' % (text, self._issue)) + info('Add comment to %s: %s' % (self._issue, text)) def add_label(self, label): """ Add the given label to the issue or PR. """ - if not label in self._labels: + if not label in self.get_labels(): self.edit(label, '--add-label') - print('Label %s added to %s' % (label, self._issue)) + info('Add label to %s: %s' % (self._issue, label)) def add_default_label(self, label): """ @@ -144,6 +175,19 @@ def on_label_add(self, label): sel_list = selection_list(label) if not sel_list: return + + if label not in self.get_labels(): + # this is possible if two labels of the same selection list + # have been added in one step (via multiple selection in the + # pull down menue). In this case `label` has been removed + # on the `on_label_add` of the first of the two labels + partn = self.active_partners(label) + if partn: + self.add_comment('Label %s can not be added due to %s!' % (label, partn[0])) + else: + warning('Label %s of %s not found!' % (label, self._issue)) + return + item = sel_list(label) if sel_list is State and self._pr: if item not in [State.needs_info, State.needs_review]: @@ -166,9 +210,9 @@ def remove_label(self, label): """ Remove the given label from the issue or PR of the handler. """ - if label in self._labels: + if label in self.get_labels(): self.edit(label, '--remove-label') - print('Label %s removed from %s' % (label, self._issue)) + info('Remove label from %s: %s' % (self._issue, label)) def on_label_remove(self, label): """ @@ -180,9 +224,22 @@ def on_label_remove(self, label): sel_list = selection_list(label) if not sel_list: return + + if label in self.get_labels(): + # this is possible if two labels of the same selection list + # have been removed in one step (via multiple selection in the + # pull down menue). In this case `label` has been added + # on the `on_label_removed` of the first of the two labels + partn = self.active_partners(label) + if not partn: + self.add_comment('Label %s can not be removed (last one of list)!' % label) + else: + self.on_label_add(partn[0]) + return + item = sel_list(label) if sel_list is State and self._pr: - if item in [State.positive_review, State.needs_work, State.close]: + if item in [State.positive_review, State.needs_work]: self.add_comment('Label can not be removed. Please use the corresponding functionality of GitHub') self.select_label(label) return @@ -201,33 +258,28 @@ def on_label_remove(self, label): # Main ############################################################################### cmdline_args = sys.argv[1:] -print('cmdline_args', len(cmdline_args), cmdline_args) -if len(cmdline_args) < 6: - print('Need 6 arguments: action, url, label, state, issue_labels, pr_labels' ) - exit -else: - action, url, label, state, issue_labels, pr_labels = cmdline_args -labels = json.loads(issue_labels) -if not labels: - labels = json.loads(pr_labels) +getLogger().setLevel(INFO) +info('cmdline_args (%s) %s' % (len(cmdline_args), cmdline_args)) -base_url = url -for i in range(3): base_url = os.path.dirname(base_url) -print("action", action) -print("url", url) -print("label", label) -print("state", state) -print("labels", labels) -print("base_url", base_url) +if len(cmdline_args) < 4: + print('Need 4 arguments: action, url, label, rev_state' ) + exit +else: + action, url, label, rev_state = cmdline_args -gh = GhLabelSynchronizer(url, labels) +info('action: %s' % action) +info('url: %s' % url) +info('label: %s' % label) +info('rev_state: %s' % rev_state) +gh = GhLabelSynchronizer(url) if action == 'opened': gh.add_default_label(Priority.major.value) gh.add_default_label(IssueType.enhancement.value) - gh.add_default_label(State.needs_review.value) + if gh.is_pull_request(): + gh.add_default_label(State.needs_veview.value) if action == 'labeled': gh.on_label_add(label) @@ -236,13 +288,13 @@ def on_label_remove(self, label): gh.on_label_remove(label) if action == 'submitted': - if state == 'approved': + if rev_state == 'approved': gh.select_label(State.positive_review.value) - if state == 'changes_requested': + if rev_state == 'changes_requested': gh.select_label(State.needs_work.value) - if state == 'commented': + if rev_state == 'commented': # just for testing gh.select_label(State.needs_work.value) diff --git a/.github/workflows/sync_labels.yml b/.github/workflows/sync_labels.yml index bdfd85cfd7..85c91b0a80 100644 --- a/.github/workflows/sync_labels.yml +++ b/.github/workflows/sync_labels.yml @@ -29,14 +29,11 @@ jobs: # Perform synchronization - name: Call script - run: ./sync_labels.py $ACTION $ISSUE_URL $PR_URL "$LABEL" "$STATE" "$ISSUE_LABELS" "$PR_LABELS" + run: ./sync_labels.py $ACTION $ISSUE_URL $PR_URL "$LABEL" "$REV_STATE" env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} ACTION: ${{ github.event.action }} - PR_URL: ${{ github.event.pull_request.html_url }} ISSUE_URL: ${{ github.event.issue.html_url }} - STATE: ${{ github.event.review.state }} + PR_URL: ${{ github.event.pull_request.html_url }} LABEL: ${{ github.event.label.name }} - ISSUE_LABELS: ${{ toJson(github.event.issue.labels.*.name) }} - PR_LABELS: ${{ toJson(github.event.pull_request.labels.*.name) }} - + REV_STATE: ${{ github.event.review.state }} From 525cf7a0179811267d65864feb884c1c27aa5f0d Mon Sep 17 00:00:00 2001 From: Sebastian Date: Mon, 6 Feb 2023 08:18:00 +0100 Subject: [PATCH 5/6] add closed and reopened events --- .github/sync_labels.py | 76 +++++++++++++++++++++++++------ .github/workflows/sync_labels.yml | 6 +-- 2 files changed, 64 insertions(+), 18 deletions(-) diff --git a/.github/sync_labels.py b/.github/sync_labels.py index 070ef34ff2..aee6f5aac8 100755 --- a/.github/sync_labels.py +++ b/.github/sync_labels.py @@ -52,17 +52,21 @@ class State(SelectionList): """ Enum for state lables. """ - positive_review = 'positive review' - needs_review = 'needs review' - needs_work = 'needs work' - needs_info = 'needs info' + positive_review = 's: positive review' + needs_review = 's: needs review' + needs_work = 's: needs work' + needs_info = 's: needs info' class IssueType(SelectionList): """ Enum for type lables. """ - bug = 'bug' - enhancement = 'enhancement' + bug = 't: bug' + enhancement = 't: enhancement' + performance = 't: performance' + refactoring = 't: refactoring' + feature = 't: feature' + tests = 't: tests' def selection_list(label): """ @@ -85,6 +89,8 @@ def __init__(self, url): """ self._url = url self._labels = None + self._draft = None + self._open = None number = os.path.basename(url) self._pr = True self._issue = 'pull request #%s' % number @@ -111,11 +117,35 @@ def view(self, key): from subprocess import check_output return loads(check_output(cmd, shell=True)) + def is_open(self): + """ + Return if the issue res. PR is open. + """ + if self._open is not None: + return self._open + if self.view('state')['state'] == 'OPEN': + self._open = True + else: + self._open = False + return self._open + + def is_draft(self): + """ + Return if the PR is a draft. + """ + if self._draft is not None: + return self._draft + if self.is_pull_request(): + self._draft = self.view('isDraft')['isDraft'] + else: + self._draft = False + return self._draft + def get_labels(self): """ Return the list of labels of the issue resp. PR. """ - if self._labels: + if self._labels is not None: return self._labels data = self.view('labels')['labels'] self._labels = [l['name'] for l in data] @@ -183,15 +213,21 @@ def on_label_add(self, label): # on the `on_label_add` of the first of the two labels partn = self.active_partners(label) if partn: - self.add_comment('Label %s can not be added due to %s!' % (label, partn[0])) + self.add_comment('Label *%s* can not be added due to *%s*!' % (label, partn[0])) else: warning('Label %s of %s not found!' % (label, self._issue)) return item = sel_list(label) if sel_list is State and self._pr: - if item not in [State.needs_info, State.needs_review]: - self.add_comment('Label can not be added. Please use the corresponding functionality of GitHub') + if not self.is_pull_request(): + if item != State.needs_info: + self.add_comment('Label *%s* can not be added to an issue. Please use it on the corresponding PR' % label) + self.remove_label(label) + return + + if item in [State.positive_review, State.needs_work]: + self.add_comment('Label *%s* can not be added. Please use the corresponding functionality of GitHub' % label) self.remove_label(label) return @@ -229,10 +265,10 @@ def on_label_remove(self, label): # this is possible if two labels of the same selection list # have been removed in one step (via multiple selection in the # pull down menue). In this case `label` has been added - # on the `on_label_removed` of the first of the two labels + # on the `on_label_remove` of the first of the two labels. partn = self.active_partners(label) if not partn: - self.add_comment('Label %s can not be removed (last one of list)!' % label) + self.add_comment('Label *%s* can not be removed (last one of list)!' % label) else: self.on_label_add(partn[0]) return @@ -240,7 +276,7 @@ def on_label_remove(self, label): item = sel_list(label) if sel_list is State and self._pr: if item in [State.positive_review, State.needs_work]: - self.add_comment('Label can not be removed. Please use the corresponding functionality of GitHub') + self.add_comment('Label *%s* can not be removed. Please use the corresponding functionality of GitHub' % label) self.select_label(label) return @@ -250,7 +286,7 @@ def on_label_remove(self, label): # add the next weaker label self.select_label(succ.value) else: - self.add_comment('Label can not be removed since there is no replacement') + self.add_comment('Label *%s* can not be removed since there is no replacement' % label) self.select_label(label) @@ -279,7 +315,17 @@ def on_label_remove(self, label): gh.add_default_label(Priority.major.value) gh.add_default_label(IssueType.enhancement.value) if gh.is_pull_request(): - gh.add_default_label(State.needs_veview.value) + if not gh.is_draft(): + gh.add_default_label(State.needs_review.value) + +if action == 'reopened': + if gh.is_pull_request(): + if not gh.is_draft(): + gh.add_default_label(State.needs_review.value) + +if action == 'closed': + for lab in State: + gh.remove_label(lab.value) if action == 'labeled': gh.on_label_add(label) diff --git a/.github/workflows/sync_labels.yml b/.github/workflows/sync_labels.yml index 85c91b0a80..2d0bd653d6 100644 --- a/.github/workflows/sync_labels.yml +++ b/.github/workflows/sync_labels.yml @@ -1,4 +1,4 @@ -# This workflow synchronizels groups of labels that correspond +# This workflow synchronizes groups of labels that correspond # to items of selection list in Trac. It controls that in each # such case there is just one label of the list present. # Furthermore in the case of the state it checks the labels @@ -10,9 +10,9 @@ on: pull_request_review: types: [submitted] issues: - types: [opened, labeled, unlabeled] + types: [opened, reopened, closed, labeled, unlabeled] pull_request: - types: [opened, labeled, unlabeled] + types: [opened, reopened, closed, labeled, unlabeled] jobs: synchronize: From 009fccebd4a669698a42eb9c7f8c722e13301580 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Wed, 22 Feb 2023 08:14:01 +0100 Subject: [PATCH 6/6] Changes according to review --- .github/sync_labels.py | 405 +++++++++++++++++++++++------- .github/workflows/sync_labels.yml | 7 +- 2 files changed, 315 insertions(+), 97 deletions(-) diff --git a/.github/sync_labels.py b/.github/sync_labels.py index aee6f5aac8..f70bc58d45 100755 --- a/.github/sync_labels.py +++ b/.github/sync_labels.py @@ -30,13 +30,18 @@ def succ(self): Return the successor of `self`. """ l = list(self.__class__) - i = l.index(self) - if i + 1 == len(l): - if i: - return l[i-1] - else: - return None - return l[i+1] + i = l.index(self) + 1 + if i >= len(l): + return None + return l[i] + +class ReviewDecision(Enum): + """ + Enum for `gh pr view` results for `reviewDecision`. + """ + changes_requested = 'CHANGES_REQUESTED' + approved = 'APPROVED' + unclear = 'COMMENTED' class Priority(SelectionList): """ @@ -53,26 +58,16 @@ class State(SelectionList): Enum for state lables. """ positive_review = 's: positive review' - needs_review = 's: needs review' needs_work = 's: needs work' + needs_review = 's: needs review' needs_info = 's: needs info' -class IssueType(SelectionList): - """ - Enum for type lables. - """ - bug = 't: bug' - enhancement = 't: enhancement' - performance = 't: performance' - refactoring = 't: refactoring' - feature = 't: feature' - tests = 't: tests' def selection_list(label): """ Return the selection list to which `label` belongs to. """ - for sel_list in [Priority, State, IssueType]: + for sel_list in [Priority, State]: for item in sel_list: if label == item.value: return sel_list @@ -83,22 +78,27 @@ class GhLabelSynchronizer: Handler for access to GitHub issue via the `gh` in the bash command line of the GitHub runner. """ - def __init__(self, url): + def __init__(self, url, actor): """ Python constructor sets the issue / PR url and list of active labels. """ self._url = url + self._actor = actor self._labels = None + self._author = None self._draft = None self._open = None + self._review_decision = None + self._reviews = None + self._commits = None + number = os.path.basename(url) self._pr = True self._issue = 'pull request #%s' % number if url.rfind('issue') != -1: self._issue = 'issue #%s' % number self._pr = False - info('Create label handler for %s' % self._issue) - + info('Create label handler for %s and actor %s' % (self._issue, self._actor)) def is_pull_request(self): """ @@ -115,7 +115,7 @@ def view(self, key): issue = 'pr' cmd = 'gh %s view %s --json %s' % (issue, self._url, key) from subprocess import check_output - return loads(check_output(cmd, shell=True)) + return loads(check_output(cmd, shell=True))[key] def is_open(self): """ @@ -123,10 +123,11 @@ def is_open(self): """ if self._open is not None: return self._open - if self.view('state')['state'] == 'OPEN': + if self.view('state') == 'OPEN': self._open = True else: self._open = False + info('Issue %s is open %s' % (self._issue, self._open)) return self._open def is_draft(self): @@ -136,9 +137,10 @@ def is_draft(self): if self._draft is not None: return self._draft if self.is_pull_request(): - self._draft = self.view('isDraft')['isDraft'] + self._draft = self.view('isDraft') else: self._draft = False + info('Issue %s is draft %s' % (self._issue, self._draft)) return self._draft def get_labels(self): @@ -147,39 +149,209 @@ def get_labels(self): """ if self._labels is not None: return self._labels - data = self.view('labels')['labels'] + data = self.view('labels') self._labels = [l['name'] for l in data] info('List of labels for %s: %s' % (self._issue, self._labels)) return self._labels - def edit(self, arg, option): + def get_author(self): """ - Perform a system call to `gh` to edit an resp. PR. + Return the author of the issue resp. PR. + """ + if self._author is not None: + return self._author + data = self.view('author') + self._author = self.view('author')['login'] + info('Author of %s: %s' % (self._issue, self._author)) + return self._author + + def get_review_decision(self): + """ + Return the reviewDecision of the PR. + """ + if not self.is_pull_request(): + return None + + if self._review_decision is not None: + return self._review_decision + + data = self.view('reviewDecision') + if data: + self._review_decision = ReviewDecision(data) + else: + self._review_decision = ReviewDecision.unclear + info('Review decision for %s: %s' % (self._issue, self._review_decision.value)) + return self._review_decision + + def get_reviews(self): + """ + Return the list of reviews of the PR. + """ + if not self.is_pull_request(): + return None + + if self._reviews is not None: + return self._reviews + + self._reviews = self.view('reviews') + info('Reviews for %s: %s' % (self._issue, self._reviews)) + return self._reviews + + def get_commits(self): + """ + Return the list of commits of the PR. + """ + if not self.is_pull_request(): + return None + + if self._commits is not None: + return self._commits + + self._commits = self.view('commits') + info('Commits for %s: %s' % (self._issue, self._commits)) + return self._commits + + def gh_cmd(self, cmd, arg, option): + """ + Perform a system call to `gh` for `cmd` to an isuue resp. PR. """ issue = 'issue' if self._pr: issue = 'pr' - cmd = 'gh %s edit %s %s "%s"' % (issue, self._url, option, arg) - os.system(cmd) + cmd_str = 'gh %s %s %s %s "%s"' % (issue, cmd, self._url, option, arg) + os.system(cmd_str) + + def edit(self, arg, option): + """ + Perform a system call to `gh` to edit an issue resp. PR. + """ + self.gh_cmd('edit', arg, option) - def active_partners(self, label): + def review(self, arg, text): + """ + Perform a system call to `gh` to review a PR. + """ + self.gh_cmd('review', arg, '-b %s' % text) + + def active_partners(self, item): """ Return the list of other labels from the selection list of the given one that are already present on the issue / PR. """ - sel_list = selection_list(label) - val = [i.value for i in sel_list] - return [l for l in self.get_labels() if l in val and not l == label] + sel_list = type(item) + partners = [i for i in sel_list if i != item and i.value in self.get_labels()] + info('Active partners of %s: %s' % (item, partners)) + return partners + + def needs_work(self, only_actor=True): + """ + Return `True` if the PR needs work. This is the case if + the review decision requests changes or if there is any + review reqesting changes. + """ + ch_req = ReviewDecision.changes_requested + rev_dec = self.get_review_decision() + if rev_dec: + if rev_dec == ch_req: + info('PR %s needs work (by decision)' % self._issue) + return True + else: + info('PR %s doesn\'t need work (by decision)' % self._issue) + return False + + revs = self.get_reviews() + if only_actor: + revs = [rev for rev in revs if rev['author']['login'] == self._actor] + if any(rev['state'] == ch_req.value for rev in revs): + info('PR %s needs work' % self._issue) + return True + info('PR %s doesn\'t need work' % self._issue) + return False + + def positive_review(self, only_actor=True): + """ + Return `True` if the PR has positive review. This is the + case if the review decision is approved or if there is any + approved review but no changes requesting one. + """ + appr = ReviewDecision.approved + rev_dec = self.get_review_decision() + if rev_dec: + if rev_dec == appr: + info('PR %s has positve review (by decision)' % self._issue) + return True + else: + info('PR %s doesn\'t have positve review (by decision)' % self._issue) + return False + + if self.needs_work(): + info('PR %s doesn\'t have positve review (needs work)' % self._issue) + return False + + revs = self.get_reviews() + if only_actor: + revs = [rev for rev in revs if rev['author']['login'] == self._actor] + if any(rev['state'] == appr.value for rev in revs): + info('PR %s has positve review (by decision)' % self._issue) + return True + info('PR %s doesn\'t have positve review (needs work)' % self._issue) + return False + + def approve_allowed(self): + """ + Return if the actor has permission to approve this PR. + """ + author = self.get_author() + revs = self.get_reviews() + if not any(rev['author']['authorAssociation'] == 'MEMBER' for rev in revs): + info('PR %s can\'t be approved because of missing member review' % (self._issue)) + return False + + revs = [rev for rev in revs if rev['author']['login'] != self._actor] + ch_req = ReviewDecision.changes_requested + if any(rev['state'] == ch_req.value for rev in revs): + info('PR %s can\'t be approved by %s since others reqest changes' % (self._issue, self._actor)) + return False + + if author != self._actor: + info('PR %s can be approved by %s' % (self._issue, self._actor)) + return True + + revs = [rev for rev in revs if rev['author']['login'] != 'github-actions'] + if not revs: + info('PR %s can\'t be approved by the author %s since no other person reviewed it' % (self._issue, self._actor)) + return False + + comts = self.get_commits() + authors = sum(com['authors'] for com in comts) + authors = [auth for auth in authors if not auth['login'] in (self._actor, 'github-actions')] + if not authors: + info('PR %s can\'t be approved by the author %s since no other person commited to it' % (self._issue, self._actor)) + return False + + info('PR %s can be approved by the author %s as co-author' % (self._issue, self._actor)) + return True + + def approve(self): + """ + Approve the PR by the actor. + """ + self.review('--approve', '%s approved this PR' % self._actor) + info('PR %s approved by %s' % (self._issue, self._actor)) + + def request_changes(self): + """ + Request changes for this PR by the actor. + """ + self.review('--request-changes', '%s requested changes for this PR' % self._actor) + info('Changes requested for PR %s by %s' % (self._issue, self._actor)) def add_comment(self, text): """ Perform a system call to `gh` to add a comment to an issue or PR. """ - issue = 'issue' - if self._pr: - issue = 'pr' - cmd = 'gh %s comment %s -b "%s"' % (issue, self._url, text) - os.system(cmd) + + self.gh_cmd('comment', text, '-b') info('Add comment to %s: %s' % (self._issue, text)) def add_label(self, label): @@ -190,57 +362,84 @@ def add_label(self, label): self.edit(label, '--add-label') info('Add label to %s: %s' % (self._issue, label)) - def add_default_label(self, label): + def add_default_label(self, item): """ Add the given label if there is no active partner. """ - if not self.active_partners(label): - self.add_label(label) + if not self.active_partners(item): + self.add_label(item.value) def on_label_add(self, label): """ - Check if the given label belongs to a selection list. - If so, remove all other labels of that list. + Check if the given label belongs to a selection list. If so, remove + all other labels of that list. In case of a state label reviews are + booked accordingly. """ sel_list = selection_list(label) if not sel_list: return + item = sel_list(label) if label not in self.get_labels(): # this is possible if two labels of the same selection list # have been added in one step (via multiple selection in the # pull down menue). In this case `label` has been removed # on the `on_label_add` of the first of the two labels - partn = self.active_partners(label) + partn = self.active_partners(item) if partn: - self.add_comment('Label *%s* can not be added due to *%s*!' % (label, partn[0])) + self.add_comment('Label *%s* can not be added due to *%s*!' % (label, partn[0].value)) else: warning('Label %s of %s not found!' % (label, self._issue)) return - item = sel_list(label) - if sel_list is State and self._pr: + if sel_list is State: if not self.is_pull_request(): if item != State.needs_info: self.add_comment('Label *%s* can not be added to an issue. Please use it on the corresponding PR' % label) self.remove_label(label) return - if item in [State.positive_review, State.needs_work]: - self.add_comment('Label *%s* can not be added. Please use the corresponding functionality of GitHub' % label) - self.remove_label(label) - return + if item == State.positive_review: + if self.approve_allowed(): + self.approve() + elif self.needs_work(): + # PR still needs work + self.add_comment('Label *%s* can not be added. Please use the corresponding functionality of GitHub' % label) + self.select_label(State.needs_work) + return + else: + # PR still needs review + self.add_comment('Label *%s* can not be added. Please use the corresponding functionality of GitHub' % label) + if self.is_draft(): + self.remove_label(label) + else: + self.select_label(State.needs_review) + return + + if item == State.needs_work: + self.request_changes() + if not self.needs_work(): + # change request of actor could not be set + self.add_comment('Label *%s* can not be added. Please use the corresponding functionality of GitHub' % label) + if self.is_draft(): + self.remove_label(label) + else: + self.select_label(State.needs_review) + return for other in sel_list: if other != item: self.remove_label(other.value) - def select_label(self, label): + def select_label(self, item): """ Add the given label and remove all others. """ - self.add_label(label) - self.on_label_add(label) + self.add_label(item.value) + sel_list = type(item) + for other in sel_list: + if other != item: + self.remove_label(other.value) def remove_label(self, label): """ @@ -252,43 +451,61 @@ def remove_label(self, label): def on_label_remove(self, label): """ - Check if the given label belongs to a selection list. If so, reject - the removement in case it represents the stat of a PR. In all other - cases add the successor of the label except if there is none or there - exists another label of the corresponding list. + Check if the given label belongs to a selection list. If so, the + successor of the label is added except there is none or there + exists another label of the corresponding list. In case of a + state label reviews are booked accordingly. """ sel_list = selection_list(label) if not sel_list: return + item = sel_list(label) if label in self.get_labels(): # this is possible if two labels of the same selection list # have been removed in one step (via multiple selection in the # pull down menue). In this case `label` has been added # on the `on_label_remove` of the first of the two labels. - partn = self.active_partners(label) - if not partn: - self.add_comment('Label *%s* can not be removed (last one of list)!' % label) - else: - self.on_label_add(partn[0]) + partn = self.active_partners(item) + if partn: + self.on_label_add(partn[0].value) return - item = sel_list(label) - if sel_list is State and self._pr: - if item in [State.positive_review, State.needs_work]: - self.add_comment('Label *%s* can not be removed. Please use the corresponding functionality of GitHub' % label) - self.select_label(label) + if sel_list is State: + if not self.is_pull_request(): + return + if item == State.positive_review: + if self.positive_review(): + self.request_changes() + self.select_label(State.needs_work) + elif self.positive_review(only_actor=False): + self.add_comment('Label *%s* can not be removed. Please use the corresponding functionality of GitHub' % label) + self.select_label(item) + elif not self.is_draft(): + self.select_label(State.needs_review) + return + elif item == State.needs_work: + if self.needs_work(only_actor=False): + self.add_comment('Label *%s* can not be removed. Please use the corresponding functionality of GitHub' % label) + self.select_label(item) + elif not self.is_draft(): + self.select_label(State.needs_review) return - if not self.active_partners(label): + if not self.active_partners(item): + # if there is no other in the same selection list + # add the next weaker label if it exists succ = sel_list(label).succ() if succ: - # add the next weaker label - self.select_label(succ.value) - else: - self.add_comment('Label *%s* can not be removed since there is no replacement' % label) - self.select_label(label) + self.select_label(succ) + def on_converted_to_draft(self): + """ + Remove all state labels. + """ + for item in State: + self.remove_label(item.value) + ############################################################################### # Main @@ -299,31 +516,25 @@ def on_label_remove(self, label): info('cmdline_args (%s) %s' % (len(cmdline_args), cmdline_args)) if len(cmdline_args) < 4: - print('Need 4 arguments: action, url, label, rev_state' ) + print('Need 5 arguments: action, url, actor, label, rev_state' ) exit else: - action, url, label, rev_state = cmdline_args + action, url, actor, label, rev_state = cmdline_args info('action: %s' % action) info('url: %s' % url) info('label: %s' % label) +info('actor: %s' % actor) info('rev_state: %s' % rev_state) -gh = GhLabelSynchronizer(url) +gh = GhLabelSynchronizer(url, actor) -if action == 'opened': - gh.add_default_label(Priority.major.value) - gh.add_default_label(IssueType.enhancement.value) +if action in ('opened', 'reopened'): if gh.is_pull_request(): if not gh.is_draft(): - gh.add_default_label(State.needs_review.value) + gh.add_default_label(State.needs_review) -if action == 'reopened': - if gh.is_pull_request(): - if not gh.is_draft(): - gh.add_default_label(State.needs_review.value) - -if action == 'closed': +if action in ('closed', 'reopened'): for lab in State: gh.remove_label(lab.value) @@ -333,16 +544,20 @@ def on_label_remove(self, label): if action == 'unlabeled': gh.on_label_remove(label) +if action == 'ready_for_review': + gh.select_label(State.needs_review) + +if action == 'converted_to_draft': + gh.on_converted_to_draft() + if action == 'submitted': if rev_state == 'approved': - gh.select_label(State.positive_review.value) + if gh.positive_review(): + gh.select_label(State.positive_review) if rev_state == 'changes_requested': - gh.select_label(State.needs_work.value) - - if rev_state == 'commented': - # just for testing - gh.select_label(State.needs_work.value) + if gh.needs_work(): + gh.select_label(State.needs_work) if action in ('review_requested', 'ready_for_review'): - gh.select_label(State.needs_review.value) + gh.select_label(State.needs_review) diff --git a/.github/workflows/sync_labels.yml b/.github/workflows/sync_labels.yml index 2d0bd653d6..4e11a94277 100644 --- a/.github/workflows/sync_labels.yml +++ b/.github/workflows/sync_labels.yml @@ -12,7 +12,9 @@ on: issues: types: [opened, reopened, closed, labeled, unlabeled] pull_request: - types: [opened, reopened, closed, labeled, unlabeled] + types: [opened, reopened, closed, ready_for_review, converted_to_draft, labeled, unlabeled] + pull_request_target: + types: [opened, reopened, closed, ready_for_review, converted_to_draft, labeled, unlabeled] jobs: synchronize: @@ -29,11 +31,12 @@ jobs: # Perform synchronization - name: Call script - run: ./sync_labels.py $ACTION $ISSUE_URL $PR_URL "$LABEL" "$REV_STATE" + run: ./sync_labels.py $ACTION $ISSUE_URL $PR_URL $ACTOR "$LABEL" "$REV_STATE" env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} ACTION: ${{ github.event.action }} ISSUE_URL: ${{ github.event.issue.html_url }} PR_URL: ${{ github.event.pull_request.html_url }} + ACTOR: ${{ github.actor }} LABEL: ${{ github.event.label.name }} REV_STATE: ${{ github.event.review.state }}