From f8d886e3c218de442998be60eed50575785759a2 Mon Sep 17 00:00:00 2001 From: Jean Jordaan Date: Mon, 24 Feb 2020 13:32:38 +0700 Subject: [PATCH 1/6] Implement merge_label, fix computing of labels_to_merge - Fix order of operations (merge, update, delete). This implementation assumes that when an old label is merged to another label, the old label should be deleted. This should normally be what is intended, and avoids ambiguity (e.g. people start adding the old label again by force of habit). --- src/labels/cli.py | 44 +++++++++++++++++++-------- src/labels/github.py | 72 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 12 deletions(-) diff --git a/src/labels/cli.py b/src/labels/cli.py index b904c82..9868233 100644 --- a/src/labels/cli.py +++ b/src/labels/cli.py @@ -164,8 +164,9 @@ def sync_cmd( On success this will also update the local labels file, so that section names match the `name` parameter. """ - labels_to_delete = {} + labels_to_merge = {} labels_to_update = {} + labels_to_delete = {} labels_to_create = {} labels_to_ignore = {} @@ -187,7 +188,11 @@ def sync_cmd( if local_label.params_dict == remote_label.params_dict: labels_to_ignore[remote_name] = local_label else: - labels_to_update[remote_name] = local_label + if (remote_name != local_label.name) and (local_label.name in remote_labels): + # There is already a label with this name + labels_to_merge[remote_name] = local_label.name + else: + labels_to_update[remote_name] = local_label else: if remote_name == local_label.name: labels_to_create[local_label.name] = local_label @@ -212,18 +217,20 @@ def sync_cmd( if dryrun: # Do not modify remote labels, but only print info dryrun_echo( - labels_to_delete, labels_to_update, labels_to_create, labels_to_ignore + labels_to_merge, labels_to_update, labels_to_delete, labels_to_create, labels_to_ignore ) - sys.exit(0) + # sys.exit(0) + return failures = [] - for name in labels_to_delete.keys(): + # Merge has to occur before update and delete + for old_label, new_label in labels_to_merge.items(): try: - context.client.delete_label(repository, name=name) + context.client.merge_label(repository, old_label=old_label, new_label=new_label) except LabelsException as exc: click.echo(str(exc), err=True) - failures.append(name) + failures.append(old_label) for name, label in labels_to_update.items(): try: @@ -232,6 +239,13 @@ def sync_cmd( click.echo(str(exc), err=True) failures.append(name) + for name in labels_to_delete.keys(): + try: + context.client.delete_label(repository, name=name) + except LabelsException as exc: + click.echo(str(exc), err=True) + failures.append(name) + for name, label in labels_to_create.items(): try: context.client.create_label(repository, label=label) @@ -253,23 +267,29 @@ def sync_cmd( def dryrun_echo( - labels_to_delete: Labels_Dict, + labels_to_merge: dict, labels_to_update: Labels_Dict, + labels_to_delete: Labels_Dict, labels_to_create: Labels_Dict, labels_to_ignore: Labels_Dict, ) -> None: """Print information about how labels would be updated on sync.""" - if labels_to_delete: - click.echo(f"This would delete the following labels:") - for name in labels_to_delete: - click.echo(f" - {name}") + if labels_to_merge: + click.echo(f"This would merge the following labels:") + for name in labels_to_merge: + click.echo(f" - {', '.join([' to '.join((old, new)) for old, new in labels_to_merge.items()])}") if labels_to_update: click.echo(f"This would update the following labels:") for name in labels_to_update: click.echo(f" - {name}") + if labels_to_delete: + click.echo(f"This would delete the following labels:") + for name in labels_to_delete: + click.echo(f" - {name}") + if labels_to_create: click.echo(f"This would create the following labels:") for name in labels_to_create: diff --git a/src/labels/github.py b/src/labels/github.py index 8996b53..eb01352 100644 --- a/src/labels/github.py +++ b/src/labels/github.py @@ -150,6 +150,78 @@ def edit_label(self, repo: Repository, *, name: str, label: Label) -> Label: return Label(**response.json()) + def merge_label(self, repo: Repository, *, old_label: str, new_label: str) -> None: + """Merge a GitHub issue label to an existing label. + + - Add the target label to all issues with the old label. + - The old label will be deleted while processing labels to delete. + """ + logger = logging.getLogger("labels") + logger.debug(f"Requesting issues for label {old_label} in {repo.owner}/{repo.name}") + + response = self.session.get( + f"{self.base_url}/search/issues?q=label:{old_label}+repo:{repo.owner}/{repo.name}", + headers={"Accept": "application/vnd.github.symmetra-preview+json"}, + ) + + if response.status_code != 200: + raise GitHubException( + f"Error retrieving issues for label {old_label} in {repo.owner}/{repo.name}: " + f"{response.status_code} - " + f"{response.reason}" + ) + + json = response.json() + + next_page = response.links.get('next', None) + while next_page: + logger.debug(f"Requesting {next_page}") + response = self.session.get( + next_page['url'], + headers={"Accept": "application/vnd.github.symmetra-preview+json"}, + ) + + if response.status_code != 200: + raise GitHubException( + f"Error retrieving next page of issues for label {old_label}: " + f"{response.status_code} - " + f"{response.reason}" + ) + + json.extend(response.json()) + next_page = response.links.get('next', None) + + for issue in json['items']: + response = self.session.get( + f"{self.base_url}/repos/{repo.owner}/{repo.name}/issues/{issue['number']}/labels", + headers={"Accept": "application/vnd.github.symmetra-preview+json"}, + ) + + if response.status_code != 200: + raise GitHubException( + f"Error retrieving labels for {repo.owner}/{repo.name}/issue/{issue['number']}: " + f"{response.status_code} - " + f"{response.reason}" + ) + + labels = [l['name'] for l in response.json()] + + if new_label not in labels: + breakpoint() + response = self.session.post( + f"{self.base_url}/repos/{repo.owner}/{repo.name}/issues/{issue['number']}/labels", + headers={"Accept": "application/vnd.github.symmetra-preview+json"}, + json={'labels': [f"{new_label}"]}, + ) + if response.status_code != 200: + raise GitHubException( + f"Error adding '{new_label}' for issue {repo.owner}/{repo.name}/issues/{issue['number']}: " + f"{response.status_code} - " + f"{response.reason}" + ) + logger.debug(f"Added label '{new_label}' to {repo.owner}/{repo.name}/issue/{issue['number']}") + + def delete_label(self, repo: Repository, *, name: str) -> None: """Delete a GitHub issue label. From cf567a06e5517ad7cf9b1f932a792f9e87ec3fd7 Mon Sep 17 00:00:00 2001 From: Jean Jordaan Date: Mon, 24 Feb 2020 14:12:43 +0700 Subject: [PATCH 2/6] Avoid line-too-long linting errors I ignored some, since I don't think splitting the URLs makes them any clearer. Happy to revert. --- src/labels/cli.py | 15 +++++++++++---- src/labels/github.py | 18 ++++++++---------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/labels/cli.py b/src/labels/cli.py index 9868233..6cb7723 100644 --- a/src/labels/cli.py +++ b/src/labels/cli.py @@ -188,7 +188,8 @@ def sync_cmd( if local_label.params_dict == remote_label.params_dict: labels_to_ignore[remote_name] = local_label else: - if (remote_name != local_label.name) and (local_label.name in remote_labels): + if ((remote_name != local_label.name) + and (local_label.name in remote_labels)): # There is already a label with this name labels_to_merge[remote_name] = local_label.name else: @@ -217,7 +218,11 @@ def sync_cmd( if dryrun: # Do not modify remote labels, but only print info dryrun_echo( - labels_to_merge, labels_to_update, labels_to_delete, labels_to_create, labels_to_ignore + labels_to_merge, + labels_to_update, + labels_to_delete, + labels_to_create, + labels_to_ignore ) # sys.exit(0) return @@ -227,7 +232,8 @@ def sync_cmd( # Merge has to occur before update and delete for old_label, new_label in labels_to_merge.items(): try: - context.client.merge_label(repository, old_label=old_label, new_label=new_label) + context.client.merge_label( + repository, old_label=old_label, new_label=new_label) except LabelsException as exc: click.echo(str(exc), err=True) failures.append(old_label) @@ -278,7 +284,8 @@ def dryrun_echo( if labels_to_merge: click.echo(f"This would merge the following labels:") for name in labels_to_merge: - click.echo(f" - {', '.join([' to '.join((old, new)) for old, new in labels_to_merge.items()])}") + click.echo(f"""\ +- {', '.join([' to '.join((old, new)) for old, new in labels_to_merge.items()])}""") if labels_to_update: click.echo(f"This would update the following labels:") diff --git a/src/labels/github.py b/src/labels/github.py index eb01352..6da509f 100644 --- a/src/labels/github.py +++ b/src/labels/github.py @@ -157,16 +157,16 @@ def merge_label(self, repo: Repository, *, old_label: str, new_label: str) -> No - The old label will be deleted while processing labels to delete. """ logger = logging.getLogger("labels") - logger.debug(f"Requesting issues for label {old_label} in {repo.owner}/{repo.name}") + logger.debug(f"Requesting issues for label {old_label} in {repo.owner}/{repo.name}") # noqa: E501 response = self.session.get( - f"{self.base_url}/search/issues?q=label:{old_label}+repo:{repo.owner}/{repo.name}", + f"{self.base_url}/search/issues?q=label:{old_label}+repo:{repo.owner}/{repo.name}", # noqa: E501 headers={"Accept": "application/vnd.github.symmetra-preview+json"}, ) if response.status_code != 200: raise GitHubException( - f"Error retrieving issues for label {old_label} in {repo.owner}/{repo.name}: " + f"Error retrieving issues for label {old_label} in {repo.owner}/{repo.name}: " # noqa: E501 f"{response.status_code} - " f"{response.reason}" ) @@ -193,13 +193,13 @@ def merge_label(self, repo: Repository, *, old_label: str, new_label: str) -> No for issue in json['items']: response = self.session.get( - f"{self.base_url}/repos/{repo.owner}/{repo.name}/issues/{issue['number']}/labels", + f"{self.base_url}/repos/{repo.owner}/{repo.name}/issues/{issue['number']}/labels", # noqa: E501 headers={"Accept": "application/vnd.github.symmetra-preview+json"}, ) if response.status_code != 200: raise GitHubException( - f"Error retrieving labels for {repo.owner}/{repo.name}/issue/{issue['number']}: " + f"Error retrieving labels for {repo.owner}/{repo.name}/issue/{issue['number']}: " # noqa: E501 f"{response.status_code} - " f"{response.reason}" ) @@ -207,20 +207,18 @@ def merge_label(self, repo: Repository, *, old_label: str, new_label: str) -> No labels = [l['name'] for l in response.json()] if new_label not in labels: - breakpoint() response = self.session.post( - f"{self.base_url}/repos/{repo.owner}/{repo.name}/issues/{issue['number']}/labels", + f"{self.base_url}/repos/{repo.owner}/{repo.name}/issues/{issue['number']}/labels", # noqa: E501 headers={"Accept": "application/vnd.github.symmetra-preview+json"}, json={'labels': [f"{new_label}"]}, ) if response.status_code != 200: raise GitHubException( - f"Error adding '{new_label}' for issue {repo.owner}/{repo.name}/issues/{issue['number']}: " + f"Error adding '{new_label}' for issue {repo.owner}/{repo.name}/issues/{issue['number']}: " # noqa: E501 f"{response.status_code} - " f"{response.reason}" ) - logger.debug(f"Added label '{new_label}' to {repo.owner}/{repo.name}/issue/{issue['number']}") - + logger.debug(f"Added label '{new_label}' to {repo.owner}/{repo.name}/issue/{issue['number']}") # noqa: E501 def delete_label(self, repo: Repository, *, name: str) -> None: """Delete a GitHub issue label. From a9387f6928654fc0f1e97989db85c7f10273cde2 Mon Sep 17 00:00:00 2001 From: Jean Jordaan Date: Mon, 24 Feb 2020 14:18:40 +0700 Subject: [PATCH 3/6] The order of execution has changed - Merge first - Then update - Then delete (need to search for to-be-deleted labels to merge) --- tests/test_cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index cae7e59..92952fb 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -194,10 +194,10 @@ def test_sync_dryrun( assert result.exit_code == 0 output = ( - "This would delete the following labels:\n" - " - infra\n" "This would update the following labels:\n" " - bug\n" + "This would delete the following labels:\n" + " - infra\n" "This would create the following labels:\n" " - dependencies\n" "This would NOT modify the following labels:\n" From 49d3283356806207a9e1adcff0bf899a9889f8f3 Mon Sep 17 00:00:00 2001 From: Jean Jordaan Date: Mon, 24 Feb 2020 14:31:15 +0700 Subject: [PATCH 4/6] Add some docs --- README.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 499db1b..2a5d99b 100644 --- a/README.md +++ b/README.md @@ -118,6 +118,8 @@ You can make the following changes to labels for your repo: labels file 🗑 - You can **edit** a label by changing the value for one or more parameters for that label 🎨 +- You can **merge** one label to another by setting the ``name`` of a label to +that of an existing label. The merged label will be deleted. - You can **create** a new label by adding a new section with your desired parameters 📝 @@ -137,15 +139,18 @@ labels sync -n -o hackebrot -r pytest-emoji ``` ```text -This would delete the following labels: - - dependencies +This would merge the following labels: + - dependencies to dependency This would update the following labels: - bug - good first issue +This would delete the following labels: + - dependencies This would create the following labels: - duplicate This would NOT modify the following labels: - code quality + - dependency - docs ``` From d5751a92367f3106c555ee557e7f6050d3b7abfe Mon Sep 17 00:00:00 2001 From: Jean Jordaan Date: Mon, 24 Feb 2020 22:01:08 +0700 Subject: [PATCH 5/6] Ignore renames of missing labels --- src/labels/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/labels/cli.py b/src/labels/cli.py index 6cb7723..55e516d 100644 --- a/src/labels/cli.py +++ b/src/labels/cli.py @@ -204,7 +204,7 @@ def sync_cmd( f'parameter: "{local_label.name}"', err=True, ) - sys.exit(1) + labels_to_ignore[remote_name] = local_label for remote_name, remote_label in remote_labels.items(): if remote_name in labels_to_update: @@ -285,7 +285,7 @@ def dryrun_echo( click.echo(f"This would merge the following labels:") for name in labels_to_merge: click.echo(f"""\ -- {', '.join([' to '.join((old, new)) for old, new in labels_to_merge.items()])}""") + - {', '.join([f"'{old}' to '{new}'" for old, new in labels_to_merge.items()])}""") if labels_to_update: click.echo(f"This would update the following labels:") From 25031a4e9d0fa6f560bf43791720dd87bbb4ec96 Mon Sep 17 00:00:00 2001 From: Jean Jordaan Date: Mon, 24 Feb 2020 22:02:54 +0700 Subject: [PATCH 6/6] Improve logging of merge process --- src/labels/github.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/labels/github.py b/src/labels/github.py index 6da509f..894651f 100644 --- a/src/labels/github.py +++ b/src/labels/github.py @@ -157,7 +157,7 @@ def merge_label(self, repo: Repository, *, old_label: str, new_label: str) -> No - The old label will be deleted while processing labels to delete. """ logger = logging.getLogger("labels") - logger.debug(f"Requesting issues for label {old_label} in {repo.owner}/{repo.name}") # noqa: E501 + logger.debug(f"Merging label '{old_label}' to '{new_label}' in {repo.owner}/{repo.name}") # noqa: E501 response = self.session.get( f"{self.base_url}/search/issues?q=label:{old_label}+repo:{repo.owner}/{repo.name}", # noqa: E501