Code review comment for lp:~al-maisan/bzr-builddeb/odd-behaviour-385667

Revision history for this message
James Westby (james-w) wrote :

> Hello there!
>
> This branch fixes
>
> - the glitch described in
> https://bugs.launchpad.net/bzr-builddeb/+bug/385667/comments/1 and
> - the problem with creating debian changelog files in empty branches

Hi,

Thanks for this.

28 + else:
29 + # changelog not in place yet.
30 + if not os.path.exists(debian_dir):
31 + os.makedirs(debian_dir)
32 + dch_args.extend(["--create", "--package", package])

is “package” guaranteed to not be None in this case?

166 + cmd.run(
167 + directory='%s/empty-branch'%workdir,
168 + location='%s/%s' % (workdir, self.upstream_tarball_name),
169 + package='test', version=self.upstream_version)

We shouldn't be calling cmd.run in the tests.

170 + # The merge-upstream will add a changelog but leave the tree in an
171 + # uncommitted state.
172 + self.assertRaises(AddChangelogError, find_changelog, tree, False)

why are you using this as the test? It doesn't seem to show
your intent with this part of the test, it feels as though you are
testing a side-effect.

Thinking again, should we be creating a changelog? There won't be
the rest of the packaging files, so you can't work with it, but using
something like dh_make will fail. Should we in fact run dh_make?

>
> The other problems described in bug #385667 either do not occur any more
> (negative upstream version numbers)

I don't know why this would have changed?

Thanks,

James

« Back to merge proposal