From 69645bd3268f55be97a60ff006d57fb74feb99d6 Mon Sep 17 00:00:00 2001 From: Jean Jordaan Date: Sun, 12 Jan 2020 12:05:14 +0200 Subject: [PATCH 01/11] Bring README into sync with code; format code blocks --- README.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 499db1b..5dca567 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ CLI app for managing GitHub labels for Python 3.6 and newer. 📝 **labels** is available for download from [PyPI][PyPI] via [pip][pip]: -```text +```bash pip install labels ``` @@ -23,6 +23,14 @@ export LABELS_USERNAME="" export LABELS_TOKEN="" ``` +You can override one or both of these values manually using the following CLI +options: + +```text +-u, --username TEXT GitHub username +-t, --token TEXT GitHub access token +``` + ## Usage Once you've installed **labels** and set up the environment variables, you're @@ -54,7 +62,7 @@ information. The default name for this file is ``labels.toml`` in your current working directory and can be changed by passing the ``-f, --filename PATH`` option followed by the path to where you want to write to. -```text +```bash labels fetch -o hackebrot -r pytest-emoji ``` From fa0d22d75b96979dbd18cdc79acc2660201930e2 Mon Sep 17 00:00:00 2001 From: Jean Jordaan Date: Sun, 12 Jan 2020 12:05:55 +0200 Subject: [PATCH 02/11] Handle multiple pages --- src/labels/github.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/labels/github.py b/src/labels/github.py index 8996b53..bcc4593 100644 --- a/src/labels/github.py +++ b/src/labels/github.py @@ -69,6 +69,20 @@ def list_labels(self, repo: Repository) -> typing.List[Label]: f"{self.base_url}/repos/{repo.owner}/{repo.name}/labels", headers={"Accept": "application/vnd.github.symmetra-preview+json"}, ) + json = response.json() + + link_header = response.headers.get('Link', []) + next_page = [l for l in link_header.split(',') if 'rel="next"' in l] + while next_page: + l, _ = next_page[0].split(';') + logger.debug(f"Requesting {l.split('?')[1]}") + response = self.session.get( + l[1:-1], + headers={"Accept": "application/vnd.github.symmetra-preview+json"}, + ) + json.extend(response.json()) + link_header = response.headers.get('Link') + next_page = [l for l in link_header.split(',') if 'rel="next"' in l] if response.status_code != 200: raise GitHubException( @@ -77,7 +91,7 @@ def list_labels(self, repo: Repository) -> typing.List[Label]: f"{response.reason}" ) - return [Label(**data) for data in response.json()] + return [Label(**data) for data in json] def get_label(self, repo: Repository, *, name: str) -> Label: """Return a single Label from the repository. From a93b59c4396b5e1967ab4d0f59152775970ecb21 Mon Sep 17 00:00:00 2001 From: Jean Jordaan Date: Sun, 12 Jan 2020 13:18:34 +0200 Subject: [PATCH 03/11] 'Link' is a string, not a list --- 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 bcc4593..b29ecf7 100644 --- a/src/labels/github.py +++ b/src/labels/github.py @@ -71,7 +71,7 @@ def list_labels(self, repo: Repository) -> typing.List[Label]: ) json = response.json() - link_header = response.headers.get('Link', []) + link_header = response.headers.get('Link', '') next_page = [l for l in link_header.split(',') if 'rel="next"' in l] while next_page: l, _ = next_page[0].split(';') From a37fa36524bc286860e378c144fab550c164cfcc Mon Sep 17 00:00:00 2001 From: Jean Jordaan Date: Sun, 12 Jan 2020 13:28:06 +0200 Subject: [PATCH 04/11] MyPy thinks 'Link' is optional MyPy thinks 'Link' is optional, although we know that if there was a 'next' link there will be a 'prev' link. --- 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 b29ecf7..d59026a 100644 --- a/src/labels/github.py +++ b/src/labels/github.py @@ -81,7 +81,7 @@ def list_labels(self, repo: Repository) -> typing.List[Label]: headers={"Accept": "application/vnd.github.symmetra-preview+json"}, ) json.extend(response.json()) - link_header = response.headers.get('Link') + link_header = response.headers.get('Link', '') next_page = [l for l in link_header.split(',') if 'rel="next"' in l] if response.status_code != 200: From f82ec6beb6a953b17795b10d65c04a45f76d7912 Mon Sep 17 00:00:00 2001 From: Jean Jordaan Date: Thu, 6 Feb 2020 22:04:34 +0700 Subject: [PATCH 05/11] Use requests's pagination support --- src/labels/github.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/labels/github.py b/src/labels/github.py index d59026a..36f5c81 100644 --- a/src/labels/github.py +++ b/src/labels/github.py @@ -71,18 +71,15 @@ def list_labels(self, repo: Repository) -> typing.List[Label]: ) json = response.json() - link_header = response.headers.get('Link', '') - next_page = [l for l in link_header.split(',') if 'rel="next"' in l] + next_page = response.links.get('next', None) while next_page: - l, _ = next_page[0].split(';') - logger.debug(f"Requesting {l.split('?')[1]}") + logger.debug(f"Requesting {next_page}") response = self.session.get( - l[1:-1], + next_page['url'], headers={"Accept": "application/vnd.github.symmetra-preview+json"}, ) json.extend(response.json()) - link_header = response.headers.get('Link', '') - next_page = [l for l in link_header.split(',') if 'rel="next"' in l] + next_page = response.links.get('next', None) if response.status_code != 200: raise GitHubException( From af1e5430fff5a31e08d9f40cce7ba87454d0219f Mon Sep 17 00:00:00 2001 From: Jean Jordaan Date: Thu, 6 Feb 2020 22:09:49 +0700 Subject: [PATCH 06/11] Don't mark terminal commands as bash script --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 5dca567..192cd1e 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ CLI app for managing GitHub labels for Python 3.6 and newer. 📝 **labels** is available for download from [PyPI][PyPI] via [pip][pip]: -```bash +```text pip install labels ``` @@ -62,7 +62,7 @@ information. The default name for this file is ``labels.toml`` in your current working directory and can be changed by passing the ``-f, --filename PATH`` option followed by the path to where you want to write to. -```bash +```text labels fetch -o hackebrot -r pytest-emoji ``` From 6a1858f0de92abda2416becf6eb81004451cf372 Mon Sep 17 00:00:00 2001 From: Jean Jordaan Date: Thu, 6 Feb 2020 22:14:11 +0700 Subject: [PATCH 07/11] Check response.status_code after each request --- src/labels/github.py | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/labels/github.py b/src/labels/github.py index 36f5c81..c4209d6 100644 --- a/src/labels/github.py +++ b/src/labels/github.py @@ -69,6 +69,14 @@ def list_labels(self, repo: Repository) -> typing.List[Label]: f"{self.base_url}/repos/{repo.owner}/{repo.name}/labels", headers={"Accept": "application/vnd.github.symmetra-preview+json"}, ) + + if response.status_code != 200: + raise GitHubException( + f"Error retrieving labels: " + f"{response.status_code} - " + f"{response.reason}" + ) + json = response.json() next_page = response.links.get('next', None) @@ -78,16 +86,17 @@ def list_labels(self, repo: Repository) -> typing.List[Label]: next_page['url'], headers={"Accept": "application/vnd.github.symmetra-preview+json"}, ) + + if response.status_code != 200: + raise GitHubException( + f"Error retrieving next page of labels: " + f"{response.status_code} - " + f"{response.reason}" + ) + json.extend(response.json()) next_page = response.links.get('next', None) - if response.status_code != 200: - raise GitHubException( - f"Error retrieving labels: " - f"{response.status_code} - " - f"{response.reason}" - ) - return [Label(**data) for data in json] def get_label(self, repo: Repository, *, name: str) -> Label: From b445e5d8a98cc74887db139d2e0394b3c2f850b5 Mon Sep 17 00:00:00 2001 From: Jean Jordaan Date: Mon, 24 Feb 2020 13:32:38 +0700 Subject: [PATCH 08/11] 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 c4209d6..965bfe3 100644 --- a/src/labels/github.py +++ b/src/labels/github.py @@ -170,6 +170,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 f9699cdcd9f6bd3b78af023bb5df70cbf26bb060 Mon Sep 17 00:00:00 2001 From: Jean Jordaan Date: Mon, 24 Feb 2020 14:12:43 +0700 Subject: [PATCH 09/11] 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 965bfe3..4f3b705 100644 --- a/src/labels/github.py +++ b/src/labels/github.py @@ -177,16 +177,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}" ) @@ -213,13 +213,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}" ) @@ -227,20 +227,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 0fdc65832b435b8bfb5daf76dec9d763e4701640 Mon Sep 17 00:00:00 2001 From: Jean Jordaan Date: Mon, 24 Feb 2020 14:18:40 +0700 Subject: [PATCH 10/11] 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 12e8442d477d383a78eaf9d6cb23a9c7d7d6915a Mon Sep 17 00:00:00 2001 From: Jean Jordaan Date: Mon, 24 Feb 2020 14:31:15 +0700 Subject: [PATCH 11/11] Add some docs --- README.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 192cd1e..2321d2d 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,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 📝 @@ -145,15 +147,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 ```