From dfe55f682722e76b2b2d3137717f72dd105fd1ab Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Thu, 29 Jan 2015 20:46:45 +0000 Subject: [PATCH] Update hacking, enable some checks and fix style issues Change-Id: Idf488af73dca3aee7c024bea4d93361be2008ce0 --- build_manpage.py | 2 +- git_upstream/commands/drop.py | 13 ++--- git_upstream/commands/import.py | 78 ++++++++++++--------------- git_upstream/commands/supersede.py | 20 +++---- git_upstream/lib/note.py | 3 +- git_upstream/lib/pygitcompat.py | 3 +- git_upstream/lib/rebaseeditor.py | 2 +- git_upstream/lib/searchers.py | 5 +- git_upstream/log.py | 2 +- git_upstream/main.py | 8 +-- git_upstream/rebase_editor.py | 13 ++--- git_upstream/tests/base.py | 4 +- git_upstream/tests/test_commands.py | 2 +- git_upstream/tests/test_drop.py | 7 +-- git_upstream/tests/test_import.py | 12 ++--- git_upstream/tests/test_import_cmd.py | 7 ++- git_upstream/tests/test_log.py | 7 +-- git_upstream/tests/test_searchers.py | 7 +-- git_upstream/tests/test_strategies.py | 4 +- git_upstream/tests/test_supersede.py | 10 ++-- test-requirements.txt | 2 +- tox.ini | 3 +- 22 files changed, 105 insertions(+), 109 deletions(-) diff --git a/build_manpage.py b/build_manpage.py index ffc9230..1b33bda 100644 --- a/build_manpage.py +++ b/build_manpage.py @@ -17,8 +17,8 @@ """ Automatically generate the man page from argparse""" -import datetime import argparse +import datetime from distutils.core import Command import os diff --git a/git_upstream/commands/drop.py b/git_upstream/commands/drop.py index 1edf6af..a0ecc02 100644 --- a/git_upstream/commands/drop.py +++ b/git_upstream/commands/drop.py @@ -15,15 +15,16 @@ # limitations under the License. # -from git_upstream.errors import GitUpstreamError -from git_upstream.log import LogDedentMixin -from git_upstream.lib.utils import GitMixin -from git_upstream import subcommand, log +import inspect +import re from git import BadObject -import inspect -import re +from git_upstream.errors import GitUpstreamError +from git_upstream.lib.utils import GitMixin +from git_upstream import log +from git_upstream.log import LogDedentMixin +from git_upstream import subcommand try: from git import BadName diff --git a/git_upstream/commands/import.py b/git_upstream/commands/import.py index b4810b0..1acf094 100644 --- a/git_upstream/commands/import.py +++ b/git_upstream/commands/import.py @@ -15,18 +15,25 @@ # limitations under the License. # -from git_upstream.errors import GitUpstreamError -from git_upstream.log import LogDedentMixin -from git_upstream.lib.utils import GitMixin -from git_upstream.lib.rebaseeditor import RebaseEditor -from git_upstream import subcommand, log -from git_upstream.lib.searchers import UpstreamMergeBaseSearcher - -from abc import ABCMeta, abstractmethod +from abc import ABCMeta +from abc import abstractmethod from collections import Sequence +import inspect + from git import GitCommandError -import inspect +from git_upstream.errors import GitUpstreamError +from git_upstream.lib.rebaseeditor import RebaseEditor +from git_upstream.lib.searchers import DiscardDuplicateGerritChangeId +from git_upstream.lib.searchers import DroppedCommitFilter +from git_upstream.lib.searchers import NoMergeCommitFilter +from git_upstream.lib.searchers import ReverseCommitFilter +from git_upstream.lib.searchers import SupersededCommitFilter +from git_upstream.lib.searchers import UpstreamMergeBaseSearcher +from git_upstream.lib.utils import GitMixin +from git_upstream import log +from git_upstream.log import LogDedentMixin +from git_upstream import subcommand class ImportUpstreamError(GitUpstreamError): @@ -35,8 +42,7 @@ class ImportUpstreamError(GitUpstreamError): class ImportUpstream(LogDedentMixin, GitMixin): - """ - Import code from an upstream project and merge in additional branches + """Import code from an upstream project and merge in additional branches to create a new branch unto which changes that are not upstream but are on the local branch are applied. """ @@ -94,16 +100,14 @@ class ImportUpstream(LogDedentMixin, GitMixin): @property def import_branch(self): - """ - Pattern to use to generate the name, or user specified branch name + """Pattern to use to generate the name, or user specified branch name to use for import. """ return self._import_branch @property def extra_branches(self): - """ - Branch containing the additional branches to be merged with the + """Branch containing the additional branches to be merged with the upstream when importing. """ return self._extra_branches @@ -139,8 +143,7 @@ class ImportUpstream(LogDedentMixin, GitMixin): def create_import(self, commit=None, import_branch=None, checkout=False, force=False): - """ - Create the import branch from the specified commit. + """Create the import branch from the specified commit. If the branch already exists abort if force is false If current branch, reset the head to the specified commit @@ -258,7 +261,7 @@ class ImportUpstream(LogDedentMixin, GitMixin): %s %s """, previous, root, branch) self.git.rebase(root, branch, onto=previous, p=True) - except: + except Exception: self.git.rebase(abort=True, with_exceptions=False) raise counter = idx - 1 @@ -298,7 +301,7 @@ class ImportUpstream(LogDedentMixin, GitMixin): try: self._linearise(self.import_branch, strategy, strategy.searcher.commit) - except: + except Exception: # Could ask user if they want to try and use the non clean route # provided they don't mind that 'git rebase --abort' will result # in a virtually useless local import branch @@ -360,9 +363,10 @@ class ImportUpstream(LogDedentMixin, GitMixin): raise NotImplementedError def finish(self): - """ - Finish merge according to the selected strategy while performing - suitable verification checks. + """Finish import + + Finish the import by merging the import branch to the target while + performing suitable verification checks. """ self.log.info("No verification checks enabled") self.git.checkout(self.branch) @@ -408,7 +412,7 @@ class ImportUpstream(LogDedentMixin, GitMixin): """, self.import_branch, self.branch) self._set_branch(self.branch, current_sha, force=True) return False - except: + except Exception: self.log.exception("Unknown exception during finish") self._set_branch(self.branch, current_sha, force=True) raise @@ -435,25 +439,18 @@ class ImportStrategiesFactory(object): return cls.__strategies.keys() -from git_upstream.lib.searchers import (NoMergeCommitFilter, - ReverseCommitFilter, - DiscardDuplicateGerritChangeId, - SupersededCommitFilter, - DroppedCommitFilter) - - class LocateChangesStrategy(GitMixin, Sequence): + """Base locate changes strategy class + + Needs to be extended with the specific strategy on how to handle changes + that are not yet upstream. """ - Base class that needs to be extended with the specific strategy on how to - handle changes locally that are not yet upstream. - """ + __metaclass__ = ABCMeta @abstractmethod def __init__(self, git=None, *args, **kwargs): - """ - Initialize an empty filters list - """ + """Initialize an empty filters list""" self.data = None self.filters = [] super(LocateChangesStrategy, self).__init__(*args, **kwargs) @@ -486,15 +483,11 @@ class LocateChangesStrategy(GitMixin, Sequence): return list(self.filtered_iter()) def _popdata(self): - """ - Should return the list of commits from the searcher object - """ + """Should return the list of commits from the searcher object""" return self.searcher.list() class LocateChangesWalk(LocateChangesStrategy): - """ - """ _strategy = "drop" @@ -565,8 +558,7 @@ class LocateChangesWalk(LocateChangesStrategy): help='Branches to additionally merge into the import branch ' 'using default git merging behaviour') def do_import(args): - """ - Import code from specified upstream branch. + """Import code from specified upstream branch. Creates an import branch from the specified upstream branch, and optionally merges additional branches given as arguments. Current branch, unless diff --git a/git_upstream/commands/supersede.py b/git_upstream/commands/supersede.py index 7db3464..2bac71e 100644 --- a/git_upstream/commands/supersede.py +++ b/git_upstream/commands/supersede.py @@ -15,18 +15,20 @@ # limitations under the License. # -from git_upstream.errors import GitUpstreamError -from git_upstream.log import LogDedentMixin -from git_upstream.lib import note # noqa -from git_upstream.lib.utils import GitMixin -from git_upstream.lib.searchers import CommitMessageSearcher -from git_upstream import subcommand, log - -from git import BadObject, Head - import inspect import re +from git import BadObject +from git import Head + +from git_upstream.errors import GitUpstreamError +from git_upstream.lib import note # noqa +from git_upstream.lib.searchers import CommitMessageSearcher +from git_upstream.lib.utils import GitMixin +from git_upstream import log +from git_upstream.log import LogDedentMixin +from git_upstream import subcommand + try: from git import BadName except ImportError: diff --git a/git_upstream/lib/note.py b/git_upstream/lib/note.py index 429c2f5..0b4fbea 100644 --- a/git_upstream/lib/note.py +++ b/git_upstream/lib/note.py @@ -15,7 +15,8 @@ # limitations under the License. # -from git import base, GitCommandError +from git import base +from git import GitCommandError from git_upstream.errors import GitUpstreamError diff --git a/git_upstream/lib/pygitcompat.py b/git_upstream/lib/pygitcompat.py index fbcb291..3a2d4f2 100644 --- a/git_upstream/lib/pygitcompat.py +++ b/git_upstream/lib/pygitcompat.py @@ -21,7 +21,8 @@ try: from git.objects.commit import Commit from git.repo import Repo except ImportError: - from git import Commit, Repo + from git import Commit + from git import Repo class GitUpstreamCompatRepo(Repo): diff --git a/git_upstream/lib/rebaseeditor.py b/git_upstream/lib/rebaseeditor.py index 0427c11..e293a41 100644 --- a/git_upstream/lib/rebaseeditor.py +++ b/git_upstream/lib/rebaseeditor.py @@ -16,8 +16,8 @@ # import codecs -import subprocess import os +import subprocess from git_upstream.lib.utils import GitMixin from git_upstream.log import LogDedentMixin diff --git a/git_upstream/lib/searchers.py b/git_upstream/lib/searchers.py index 3999c6d..4d4d3d6 100644 --- a/git_upstream/lib/searchers.py +++ b/git_upstream/lib/searchers.py @@ -15,11 +15,12 @@ # limitations under the License. # -from abc import ABCMeta, abstractmethod +from abc import ABCMeta +from abc import abstractmethod import re -from git_upstream.lib.utils import GitMixin from git_upstream.lib.pygitcompat import Commit +from git_upstream.lib.utils import GitMixin from git_upstream.log import LogDedentMixin diff --git a/git_upstream/log.py b/git_upstream/log.py index 67cc5e2..8d4ce20 100644 --- a/git_upstream/log.py +++ b/git_upstream/log.py @@ -24,8 +24,8 @@ functions for verbose/quiet CLI args to retreive the appropriate level for logging output to the console. """ -import logging from functools import wraps +import logging import textwrap diff --git a/git_upstream/main.py b/git_upstream/main.py index d789dba..0bb06df 100644 --- a/git_upstream/main.py +++ b/git_upstream/main.py @@ -20,17 +20,17 @@ Command-line tool for tracking upstream revisions """ +import argparse import logging import sys -import argparse import git +from git_upstream import __version__ from git_upstream import commands +from git_upstream.errors import GitUpstreamError from git_upstream import log from git_upstream import subcommand -from git_upstream import __version__ -from git_upstream.errors import GitUpstreamError try: import argcomplete @@ -168,7 +168,7 @@ def main(argv=None): try: args.func(args) - except GitUpstreamError, e: + except GitUpstreamError as e: logger.fatal("%s", e[0]) logger.debug("Git-Upstream: %s", e[0], exc_info=e) sys.exit(1) diff --git a/git_upstream/rebase_editor.py b/git_upstream/rebase_editor.py index 4c95c65..6bae45d 100755 --- a/git_upstream/rebase_editor.py +++ b/git_upstream/rebase_editor.py @@ -50,12 +50,12 @@ def rebase_replace_insn(path, istream): if not replacement: break if not replacement.startswith("#"): - print replacement - print "" + print(replacement) + print("") echo_out = True continue if echo_out: - print stripped + print(stripped) def main(): @@ -84,14 +84,14 @@ def main(): # process through 'git-rebase' and this script, as many editors will # have problems if stdin is a pipe. if VERBOSE: - print "rebase-editor: Replacing contents of rebase instructions file" + print("rebase-editor: Replacing contents of rebase instructions file") rebase_replace_insn(args.ofile, open(args.ifile, 'r')) # if interactive mode, attempt to exec the editor defined by the user # for use with git if not args.interactive: if VERBOSE: - print "rebase-editor: Interactive mode not enabled" + print("rebase-editor: Interactive mode not enabled") sys.exit(0) # calling code should only override one of the two editor variables, @@ -112,9 +112,6 @@ def main(): sys.stdin.flush() sys.stdout.flush() sys.stderr.flush() - #os.dup2(sys.stdin.fileno(), 0) - #os.dup2(sys.stdout.fileno(), 1) - #os.dup2(sys.stderr.fileno(), 2) os.execvpe(editor, editor_args, env=env) sys.stderr.write("rebase-editor: No git EDITOR variables defined in " diff --git a/git_upstream/tests/base.py b/git_upstream/tests/base.py index 76f3c5d..70c70d8 100644 --- a/git_upstream/tests/base.py +++ b/git_upstream/tests/base.py @@ -14,13 +14,13 @@ # under the License. import logging -import re import os +from pprint import pformat +import re import tempfile import fixtures import git -from pprint import pformat import testtools from testtools.content import text_content diff --git a/git_upstream/tests/test_commands.py b/git_upstream/tests/test_commands.py index 393a526..5bb246a 100644 --- a/git_upstream/tests/test_commands.py +++ b/git_upstream/tests/test_commands.py @@ -15,8 +15,8 @@ """Tests for the 'commands' module""" -import testtools from argparse import ArgumentParser +import testtools from git_upstream import commands as c diff --git a/git_upstream/tests/test_drop.py b/git_upstream/tests/test_drop.py index 618105f..56f1fdf 100644 --- a/git_upstream/tests/test_drop.py +++ b/git_upstream/tests/test_drop.py @@ -15,9 +15,10 @@ """Tests the drop module""" +from git import GitCommandError + from git_upstream.commands import drop as d from git_upstream.tests import base -from git import GitCommandError class TestDrop(base.BaseTestCase): @@ -37,10 +38,10 @@ class TestDrop(base.BaseTestCase): automatic_author = '%s <%s>' % (self.repo.git.config('user.name'), self.repo.git.config('user.email')) t = d.Drop(git_object=self.first_commit) - self.assertEquals(t.author, automatic_author) + self.assertEqual(t.author, automatic_author) t = d.Drop(git_object=self.first_commit, author=self.author) - self.assertEquals(t.author, self.author) + self.assertEqual(t.author, self.author) def test_invalid_commit(self): """Test drop initialization with invalid commit""" diff --git a/git_upstream/tests/test_import.py b/git_upstream/tests/test_import.py index 565e87b..8b806ec 100644 --- a/git_upstream/tests/test_import.py +++ b/git_upstream/tests/test_import.py @@ -60,9 +60,9 @@ class TestImport(BaseTestCase): self._build_git_tree(tree, branches.values()) iu = ImportUpstream("master", "upstream/master", "import") iu.finish() - self.assertEquals("", self.git.status(porcelain=True), - "ImportUpstream.finish() failed to result in a " - "clean working tree and index") + self.assertEqual("", self.git.status(porcelain=True), + "ImportUpstream.finish() failed to result in a " + "clean working tree and index") def test_import_finish_merge_extra_files(self): """Test that after finishing the import merge when the users working @@ -99,6 +99,6 @@ class TestImport(BaseTestCase): # create a dummy file open('dummy-file', 'a').close() iu.finish() - self.assertEquals("?? dummy-file", self.git.status(porcelain=True), - "ImportUpstream.finish() failed to leave user " - "files not managed untouched.") + self.assertEqual("?? dummy-file", self.git.status(porcelain=True), + "ImportUpstream.finish() failed to leave user " + "files not managed untouched.") diff --git a/git_upstream/tests/test_import_cmd.py b/git_upstream/tests/test_import_cmd.py index 863361b..9a3eeae 100644 --- a/git_upstream/tests/test_import_cmd.py +++ b/git_upstream/tests/test_import_cmd.py @@ -16,18 +16,17 @@ """Tests for the 'import' command""" import inspect +from string import lower import mock from testtools.matchers import Equals -from git_upstream import main from git_upstream.lib.pygitcompat import Commit +from git_upstream import main from git_upstream.tests.base import BaseTestCase -from string import lower - -class SubstringMatcher(): +class SubstringMatcher(object): def __init__(self, containing): self.containing = lower(containing) diff --git a/git_upstream/tests/test_log.py b/git_upstream/tests/test_log.py index e58bdc7..ea3d091 100644 --- a/git_upstream/tests/test_log.py +++ b/git_upstream/tests/test_log.py @@ -16,6 +16,7 @@ """Tests for then 'log' module""" import testtools + from git_upstream import log as l @@ -27,14 +28,14 @@ class TestGetLogger(testtools.TestCase): logger = l.get_logger() self.assertIsNotNone(logger) - self.assertEquals('git-upstream', logger.name) + self.assertEqual('git-upstream', logger.name) def test_logger_name_param(self): """Test custom logger name""" logger = l.get_logger('test') self.assertIsNotNone(logger) - self.assertEquals('git-upstream.test', logger.name) + self.assertEqual('git-upstream.test', logger.name) class TestGetIncrementLevel(testtools.TestCase): @@ -55,7 +56,7 @@ class TestGetIncrementLevel(testtools.TestCase): for level_no in range(levels - increment): for level in self._levels[level_no]: result = l.get_increment_level(1, level) - self.assertEquals( + self.assertEqual( self._levels[min(level_no + 1, levels - 1)][0].upper(), result) diff --git a/git_upstream/tests/test_searchers.py b/git_upstream/tests/test_searchers.py index 07bf485..df7b7b3 100644 --- a/git_upstream/tests/test_searchers.py +++ b/git_upstream/tests/test_searchers.py @@ -14,9 +14,10 @@ # limitations under the License. # -from base import BaseTestCase from git_upstream.lib.searchers import UpstreamMergeBaseSearcher +from base import BaseTestCase + class TestUpstreamMergeBaseSearcher(BaseTestCase): @@ -28,8 +29,8 @@ class TestUpstreamMergeBaseSearcher(BaseTestCase): searcher = UpstreamMergeBaseSearcher(branch=branches['head'][0], patterns=pattern, repo=self.repo) - self.assertEquals(self._commits_from_nodes(reversed(expected_nodes)), - searcher.list()) + self.assertEqual(self._commits_from_nodes(reversed(expected_nodes)), + searcher.list()) def test_search_basic(self): """Construct a basic repo layout and validate that locate changes diff --git a/git_upstream/tests/test_strategies.py b/git_upstream/tests/test_strategies.py index 69711dd..f70b795 100644 --- a/git_upstream/tests/test_strategies.py +++ b/git_upstream/tests/test_strategies.py @@ -30,8 +30,8 @@ class TestStrategies(BaseTestCase): strategy = LocateChangesWalk(branch=branches['head'][0], search_refs=[branches['upstream'][0]]) - self.assertEquals(self._commits_from_nodes(expected_nodes), - [c for c in strategy.filtered_iter()]) + self.assertEqual(self._commits_from_nodes(expected_nodes), + [c for c in strategy.filtered_iter()]) def test_locate_changes_walk_basic(self): """Construct a basic repo layout and validate that locate changes diff --git a/git_upstream/tests/test_supersede.py b/git_upstream/tests/test_supersede.py index b0d4491..b60369e 100644 --- a/git_upstream/tests/test_supersede.py +++ b/git_upstream/tests/test_supersede.py @@ -15,9 +15,10 @@ """Tests the supersede module""" +from git import GitCommandError + from git_upstream.commands import supersede as s from git_upstream.tests import base -from git import GitCommandError class TestSupersede(base.BaseTestCase): @@ -49,7 +50,7 @@ class TestSupersede(base.BaseTestCase): change_ids=self.first_change_ids, upstream_branch=self.change_ids_branch) - self.assertEquals(t.commit, self.first_commit) + self.assertEqual(t.commit, self.first_commit) self.assertNotEqual(t.commit, self.second_commit) self.assertEqual(str(t.change_ids_branch), self.change_ids_branch) @@ -71,7 +72,7 @@ class TestSupersede(base.BaseTestCase): change_ids=self.second_change_ids, upstream_branch=self.change_ids_branch) - self.assertEquals(t.commit, self.first_commit) + self.assertEqual(t.commit, self.first_commit) self.assertNotEqual(t.commit, self.second_commit) def test_invalid_cids(self): @@ -88,8 +89,7 @@ class TestSupersede(base.BaseTestCase): self.assertRaises(s.SupersedeError, s.Supersede, git_object=self.first_commit, change_ids=self.invalid_change_ids, - upstream_branch= - self.invalid_change_ids_branch) + upstream_branch=self.invalid_change_ids_branch) def test_no_upstream_branch(self): """Test supersede initialization with invalid branch name""" diff --git a/test-requirements.txt b/test-requirements.txt index f2efc91..5e3c51d 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,4 +1,4 @@ -hacking>=0.5.6,<0.8 +hacking>=0.9,<=0.10.0 mock Sphinx>=1.1.2,<1.2 discover diff --git a/tox.ini b/tox.ini index 0e766f1..5619932 100644 --- a/tox.ini +++ b/tox.ini @@ -25,7 +25,6 @@ commands = python setup.py build_manpage commands = {posargs} [flake8] -# H is intentionally ignored -ignore = H +ignore=H236,H40 show-source = True exclude = .venv,.tox,dist,doc,build,*.egg