forked from svigerske/trac-to-github
-
Notifications
You must be signed in to change notification settings - Fork 4
New workflow for synchronization of labels #187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
3fb0240
sync_labels initial
soehms e4f6d3b
remove unused labels
soehms c482480
adapt the defaults
soehms 1cbd42f
Merge branch 'master' of github.com:soehms/trac-to-github
soehms a0e0753
use label list via gh view
soehms 56974b5
Merge branch 'sagemath:master' into master
soehms 525cf7a
add closed and reopened events
soehms d29b3e6
Merge branch 'master' of github.com:sagemath/trac-to-github
soehms 009fcce
Changes according to review
soehms File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,348 @@ | ||
| #!/usr/bin/python | ||
| # -*- coding: utf-8 -*- | ||
|
|
||
| r""" | ||
| Python script to sync labels that are migrated from Trac selection lists. | ||
| """ | ||
|
|
||
| ############################################################################## | ||
| # Copyright (C) 2023 Sebastian Oehms <[email protected]> | ||
| # | ||
| # 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 | ||
| from logging import info, warning, getLogger, INFO | ||
| from json import loads | ||
| 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. | ||
| """ | ||
| 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 = '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 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): | ||
| """ | ||
| Python constructor sets the issue / PR url and list of active labels. | ||
| """ | ||
| 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 | ||
| if url.rfind('issue') != -1: | ||
| self._issue = 'issue #%s' % number | ||
| self._pr = False | ||
| info('Create label handler for %s' % self._issue) | ||
|
|
||
|
|
||
| def is_pull_request(self): | ||
| """ | ||
| Return if we are treating a pull request. | ||
| """ | ||
| 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 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 is not None: | ||
| 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 resp. PR. | ||
| """ | ||
| issue = 'issue' | ||
| if self._pr: | ||
| issue = 'pr' | ||
| 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. | ||
| """ | ||
| issue = 'issue' | ||
| if self._pr: | ||
| issue = 'pr' | ||
| cmd = 'gh %s comment %s -b "%s"' % (issue, self._url, text) | ||
| os.system(cmd) | ||
| 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.get_labels(): | ||
| self.edit(label, '--add-label') | ||
| info('Add label to %s: %s' % (self._issue, label)) | ||
|
|
||
| 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 | ||
|
|
||
| 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 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 | ||
|
|
||
| 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.get_labels(): | ||
| self.edit(label, '--remove-label') | ||
| info('Remove label from %s: %s' % (self._issue, 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. | ||
| """ | ||
| 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_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]) | ||
| 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) | ||
| 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 *%s* can not be removed since there is no replacement' % label) | ||
| self.select_label(label) | ||
|
|
||
|
|
||
| ############################################################################### | ||
| # Main | ||
| ############################################################################### | ||
| cmdline_args = sys.argv[1:] | ||
|
|
||
| getLogger().setLevel(INFO) | ||
| info('cmdline_args (%s) %s' % (len(cmdline_args), cmdline_args)) | ||
|
|
||
| if len(cmdline_args) < 4: | ||
| print('Need 4 arguments: action, url, label, rev_state' ) | ||
| exit | ||
| else: | ||
| action, url, label, rev_state = cmdline_args | ||
|
|
||
| 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) | ||
tobiasdiez marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| gh.add_default_label(IssueType.enhancement.value) | ||
| if gh.is_pull_request(): | ||
| 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) | ||
tobiasdiez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if action == 'labeled': | ||
| gh.on_label_add(label) | ||
|
|
||
| if action == 'unlabeled': | ||
| gh.on_label_remove(label) | ||
|
|
||
| if action == 'submitted': | ||
| if rev_state == 'approved': | ||
| gh.select_label(State.positive_review.value) | ||
tobiasdiez marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| 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 action in ('review_requested', 'ready_for_review'): | ||
| gh.select_label(State.needs_review.value) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| # 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 | ||
| # to coincide with the corresponding review state. | ||
|
|
||
| name: Synchronize selection list lables | ||
|
|
||
| on: | ||
| pull_request_review: | ||
| types: [submitted] | ||
| issues: | ||
| types: [opened, reopened, closed, labeled, unlabeled] | ||
| pull_request: | ||
tobiasdiez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| types: [opened, reopened, closed, 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/"$REPO"/master/.github/sync_labels.py" | ||
| chmod a+x sync_labels.py | ||
| env: | ||
| REPO: ${{ github.repository }} | ||
|
|
||
| # Perform synchronization | ||
| - name: Call script | ||
| run: ./sync_labels.py $ACTION $ISSUE_URL $PR_URL "$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 }} | ||
| LABEL: ${{ github.event.label.name }} | ||
| REV_STATE: ${{ github.event.review.state }} | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would still allow people to actively overwrite the label. For example, if the PR creator realizes she still needs to fix something small, then she can simply add a "needs work" label (but you cannot "request changes" on your own PR). Similarly, not every positive review means that it can directly be merged (e.g. second pairs of eyes needed).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. Perhaps we should not think "positive review" == "approval". Adding "positive review" label is one person's opinion. She may be another person who gives approval to the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with respect to
needs work. Your second point I don't understand: How has second pairs of eyes needed be realized in Trac other than selectingpositive reviewat the end by the second pairs of eyes?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I said above: I think the meaning of
positive reviewshould be as closed to the meaning we are used to from Trac. If we leave the meaning to one person's opinion then the meaning is unknown and the label is not only useless but confusing. Then, as suggested before, we better should remove it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On trac, you can add yourself as reviewer and comment that you agree with the changes, but don't set the ticket to "positive review". Something like this should still be possible on github, i.e. you review a PR positively but don't add the "positive review" label (or remove it if it has been automatically added).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I always allow to add the
needs_worklabel. This adds a review ofgithub-actionrequesting changes. Addingpositive_reviewI allow in the following cases (not sure if they are strong enought and cover what you meant by second pairs of eyes):a. There must be at least one review of someone else
b. There must be at least one commit of someone else
If all conditions are satisfied the PR will be approved by
github-action. Else the label is removed and a comment: Label can not be added. Please use the corresponding functionality of GitHub is posted.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds very reasonable! Concerning "There should not be changes requested by some one else" not sure what happens in the case someone is against the changes but multiple other people give their positive review. But maybe this situation is rare enough to not need special treatment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What should be possible is: after giving a positive review, remove the label "s: positive review" and add instead the "needs review" label (meaning "I agree with the changes, but someone else should also have a look at this PR").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think such a conflict should be resolved using the GitHub review functionality, since comments on the respective review that request changes are needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can always add
s: needs reviewands: needs info. In both cases thes: positive reviewwill be removed by the bot.