Code review comment for lp:~barry/bzr/609186-shortcuts

Revision history for this message
Martin Pool (mbp) wrote :

You should mention the short form in the documentation.

If the series are defined in here only for the sake of abbreviations, maybe you can delete the 'natty: natty' entries? Saying we know about certain abbreviations is cleaner for me than saying we know about all series. Probably that should be done on the server, but it's a bit harder to change that than to change bzr.

Just as a style point (and you don't have to change it now) I would have defined a helper method to do the check rather than copy&pasting

 def test_debian_default_distroseries_expansion(self):
322 + factory = self._make_factory(package='foo', distro='debian')
323 + self.assertEqual(
324 + 'http://bazaar.launchpad.net/~branch/debian/foo',
325 + self.directory._resolve('debianlp:foo', factory))

Instead something like

  self.checkExpansion('foo', 'debian', 'debianlp:foo', 'http://bazaar.launchpad.net/~branch/debian/foo')
325 + self.directory._resolve('debianlp:foo',

review: Needs Fixing

« Back to merge proposal