From cdffa83191f6e810f18965dda104eb0bc83e65ee Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Thu, 10 Aug 2023 13:21:21 +0200 Subject: [PATCH] [IMP] runbot_merge, forwardport: minor cleanups Remove unused imports, unnecessary f-strings, dead code, fix less-than-ideal operators. --- forwardport/models/project.py | 3 +-- forwardport/tests/test_limit.py | 4 ---- runbot_merge/github.py | 2 +- .../migrations/13.0.1.6/pre-migration.py | 3 --- runbot_merge/models/pull_requests.py | 1 - runbot_merge/sentry.py | 2 +- runbot_merge/tests/test_basic.py | 18 +++++++++--------- runbot_merge/tests/test_disabled_branch.py | 2 +- runbot_merge/tests/test_oddities.py | 2 +- runbot_merge/tests/test_provisioning.py | 4 ++-- 10 files changed, 16 insertions(+), 25 deletions(-) diff --git a/forwardport/models/project.py b/forwardport/models/project.py index 51974020..cf60f68a 100644 --- a/forwardport/models/project.py +++ b/forwardport/models/project.py @@ -13,7 +13,6 @@ it up), ... """ import ast import base64 -import collections import contextlib import datetime import itertools @@ -421,7 +420,7 @@ class PullRequests(models.Model): limit = next(tokens, None) ping = True if not self._pr_acl(author).is_author: - msg = "you can't set a forward-port limit.".format(login) + msg = "you can't set a forward-port limit." elif not limit: msg = "please provide a branch to forward-port to." else: diff --git a/forwardport/tests/test_limit.py b/forwardport/tests/test_limit.py index db13f535..ecdfd585 100644 --- a/forwardport/tests/test_limit.py +++ b/forwardport/tests/test_limit.py @@ -1,8 +1,4 @@ -# -*- coding: utf-8 -*- import collections -import time - -import pytest from utils import seen, Commit, make_basic diff --git a/runbot_merge/github.py b/runbot_merge/github.py index e5e5fa9e..5b11033b 100644 --- a/runbot_merge/github.py +++ b/runbot_merge/github.py @@ -71,7 +71,7 @@ class GH(object): """ req = response.request url = werkzeug.urls.url_parse(req.url) - if url.netloc is not 'api.github.com': + if url.netloc != 'api.github.com': return body = '' if not req.body else ('\n' + pprint.pformat(json.loads(req.body.decode()), indent=4)) diff --git a/runbot_merge/migrations/13.0.1.6/pre-migration.py b/runbot_merge/migrations/13.0.1.6/pre-migration.py index a7903018..639c5013 100644 --- a/runbot_merge/migrations/13.0.1.6/pre-migration.py +++ b/runbot_merge/migrations/13.0.1.6/pre-migration.py @@ -1,6 +1,3 @@ -import collections - - def migrate(cr, version): """ Status overrides: o2m -> m2m """ diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index 18294ea9..2873dbcf 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -1682,7 +1682,6 @@ class FeedbackTemplate(models.Model): except Exception: _logger.exception("Failed to render template %s", self.get_external_id()) raise - return None class Commit(models.Model): """Represents a commit onto which statuses might be posted, diff --git a/runbot_merge/sentry.py b/runbot_merge/sentry.py index 9c249e9e..48bdd48e 100644 --- a/runbot_merge/sentry.py +++ b/runbot_merge/sentry.py @@ -9,7 +9,7 @@ from odoo import http from odoo.addons.base.models.ir_cron import ir_cron from odoo.http import WebRequest -from .exceptions import FastForwardError, Mismatch, MergeError, Unmergeable +from .exceptions import FastForwardError, MergeError, Unmergeable def delegate(self, attr): diff --git a/runbot_merge/tests/test_basic.py b/runbot_merge/tests/test_basic.py index 305a79e2..ccc8e36f 100644 --- a/runbot_merge/tests/test_basic.py +++ b/runbot_merge/tests/test_basic.py @@ -7,7 +7,7 @@ from unittest import mock import pytest import requests -from lxml import html, etree +from lxml import html import odoo from utils import _simple_init, seen, re_matches, get_partner, Commit, pr_page, to_pr, part_of @@ -1776,9 +1776,9 @@ commits, I need to know how to merge it: with repo: root = repo.make_commits(None, Commit("root", tree={'a': 'a'}), ref='heads/master') - repo.make_commits(root, Commit('C', tree={'a': 'b'}), ref=f'heads/change') + repo.make_commits(root, Commit('C', tree={'a': 'b'}), ref='heads/change') pr = repo.make_pr(title="title", body=f'first\n{separator}\nsecond', - target='master', head=f'change') + target='master', head='change') repo.post_status(pr.head, 'success', 'legal/cla') repo.post_status(pr.head, 'success', 'ci/runbot') pr.post_comment('hansen r+ merge', config['role_reviewer']['token']) @@ -1806,7 +1806,7 @@ commits, I need to know how to merge it: with repo: root = repo.make_commits(None, Commit("root", tree={'a': 'a'}), ref='heads/master') - repo.make_commits(root, Commit('C', tree={'a': 'b'}), ref=f'heads/change') + repo.make_commits(root, Commit('C', tree={'a': 'b'}), ref='heads/change') pr = repo.make_pr(title="title", body="""\ Title --- @@ -1818,7 +1818,7 @@ This is more text *** removed """, - target='master', head=f'change') + target='master', head='change') repo.post_status(pr.head, 'success', 'legal/cla') repo.post_status(pr.head, 'success', 'ci/runbot') pr.post_comment('hansen r+ merge', config['role_reviewer']['token']) @@ -1853,8 +1853,8 @@ removed with repo: root = repo.make_commits(None, Commit("root", tree={'a': 'a'}), ref='heads/master') - repo.make_commits(root, Commit('Commit\n\nfirst\n***\nsecond', tree={'a': 'b'}), ref=f'heads/change') - pr = repo.make_pr(title="PR", body=f'first\n***\nsecond', + repo.make_commits(root, Commit('Commit\n\nfirst\n***\nsecond', tree={'a': 'b'}), ref='heads/change') + pr = repo.make_pr(title="PR", body='first\n***\nsecond', target='master', head='change') repo.post_status(pr.head, 'success', 'legal/cla') repo.post_status(pr.head, 'success', 'ci/runbot') @@ -2007,7 +2007,7 @@ Part-of: {pr_id.display_name}""" assert log_to_node(repo.log('heads/master')), expected def test_squash_merge(self, repo, env, config, users): - other_user = requests.get(f'https://api.github.com/user', headers={ + other_user = requests.get('https://api.github.com/user', headers={ 'Authorization': 'token %s' % config['role_other']['token'], }).json() other_user = { @@ -2089,7 +2089,7 @@ Signed-off-by: {get_partner(env, users["reviewer"]).formatted_email}\ # FIXME: should probably get the token from the project to be sure it's # the bot user - current_user = repo._session.get(f'https://api.github.com/user').json() + current_user = repo._session.get('https://api.github.com/user').json() current_user = { 'name': current_user['name'] or current_user['login'], # FIXME: not guaranteed diff --git a/runbot_merge/tests/test_disabled_branch.py b/runbot_merge/tests/test_disabled_branch.py index 2cff73ce..17282985 100644 --- a/runbot_merge/tests/test_disabled_branch.py +++ b/runbot_merge/tests/test_disabled_branch.py @@ -50,7 +50,7 @@ def test_existing_pr_disabled_branch(env, project, make_repo, setreviewers, conf assert not branch_id.active_staging_id assert staging_id.state == 'cancelled', \ "closing the PRs should have canceled the staging" - assert staging_id.reason == f"Target branch deactivated by 'admin'." + assert staging_id.reason == "Target branch deactivated by 'admin'." p = pr_page(page, pr) target = dict(zip( diff --git a/runbot_merge/tests/test_oddities.py b/runbot_merge/tests/test_oddities.py index f0a501ce..d8121629 100644 --- a/runbot_merge/tests/test_oddities.py +++ b/runbot_merge/tests/test_oddities.py @@ -1,6 +1,6 @@ import requests -from utils import Commit, to_pr, seen +from utils import Commit, to_pr def test_partner_merge(env): diff --git a/runbot_merge/tests/test_provisioning.py b/runbot_merge/tests/test_provisioning.py index db26e5e7..7cadcd52 100644 --- a/runbot_merge/tests/test_provisioning.py +++ b/runbot_merge/tests/test_provisioning.py @@ -179,7 +179,7 @@ def test_bulk_ops(env, port): 'email': "coc@example.org", 'active': False, }]) - assert a.active == b.active == False + assert a.active is b.active is False assert provision_user(port, [ {'email': 'bob@example.org', 'github_login': 'xyz'}, @@ -187,7 +187,7 @@ def test_bulk_ops(env, port): ]) == [2, 0] assert a.users_id assert b.users_id - assert a.active == b.active == True + assert a.active is b.active is True def provision_user(port, users): r = requests.post(f'http://localhost:{port}/runbot_merge/provision', json={