Code review comment for lp:~vila/bzr/323111-deprecate-get-backup-name

On 11 September 2010 00:44, Vincent Ladeuil <email address hidden> wrote:
> -class TestGetBackupName(TestCase):
> -    def test_get_backup_name(self):
> +class TestBackupName(tests.TestCase):
> +
> +    def test_deprecations(self):
> +        class MockTransform(object):
> +
> +            def has_named_child(self, by_parent, parent_id, name):
> +                return name in by_parent.get(parent_id, [])
> +
> +        class MockEntry(object):
> +
> +            def __init__(self):
> +                object.__init__(self)
> +                self.name = "name"
> +

Generally I'd only put the mock object inside the method if I was sure
it would only be used from there, which is probably true now this is
deprecated.

> +        name2 = self.applyDeprecated(
> +                symbol_versioning.deprecated_in((2, 3, 0)),
> +                transform._get_backup_name, 'name', {'a':['name.~1~']}, 'a', tt)
> +        self.assertEqual('name.~2~', name2)

I like the style insisted upon by Launchpad of using only 4-character
indents for continuation lines... I think we say it's not a big deal
and it's not.

> +@deprecated_function(deprecated_in((2, 3, 0)))
>  def _get_backup_name(name, by_parent, parent_trans_id, tt):
>     """Produce a backup-style name that appears to be available"""
>     def name_gen():
> @@ -2697,9 +2722,8 @@
>                         tt.delete_contents(trans_id)
>                     elif kind[1] is not None:
>                         parent_trans_id = tt.trans_id_file_id(parent[0])
> -                        by_parent = tt.by_parent()
> -                        backup_name = _get_backup_name(name[0], by_parent,
> -                                                       parent_trans_id, tt)
> +                        backup_name = tt._available_backup_name(name[0],
> +                                                                parent_trans_id)

This to me is a clear case where the other indent rule is cleaner.
<https://dev.launchpad.net/PythonStyleGuide#Multiline+function+calls>

The substance seems fine.

--
Martin