Code review comment for lp:~abentley/juju-core/sign-branch

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

>> + if not (filename.endswith('.sjson') or +
>> filename.endswith('.json.gpg')): + continue +
>> os.unlink(os.path.join(dirpath, filename))
>
> I don't think we need the continue statement when the if-statement
> looks for truth if (filename.endswith('.sjson') or
> filename.endswith('.json.gpg')): os.unlink(os.path.join(dirpath,
> filename))

Okay.

>> + def sign_branch(self): + self.bzr.bzr('checkout',
>> self.signed_branch, self.temp_branch) + self.merge() +
>> self.bzr.bzr('commit', self.temp_branch, '-m', 'Merged unsigned
>> json') + self.sign_metadata() + self.bzr.bzr('add',
>> self.temp_branch)
>
> If we were to stop making the ":" files, would bzr raise an error
> if it saw the signed ":" missing?

No. The sjson & gpg would be deleted by loop you commented on
previously. When Bazaar commits, it will notice that several
versioned files have been deleted, and unversion them.

We wouldn't want to use "bzr rm" instead of os.unlink, because in most
cases, the sjson will be replaced and we should not consider the
replacement to be a brand-new-file.

There isn't, AFAICT, a bzr command to "bzr rm" any files that have
already been deleted.

Of course, I could write this as a bzrlib client, but I thought it
would be shorter and easier to maintain using the commandline.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJWsQyKAAoJEK84cMOcf+9heHoIAKOUAYNue6nlq3pvjPQ++O8y
/jtnHaq0OvLccG1+skfvnOC5la27n/bEYdwgeVovxHo8UY6Xrf3CAY6vT9hnlyTd
3noWqd2tJLogv19ia9XNQwtvLm/ocFyAVTD1XYDZh7tgq1/Rhb3UFwFMhEstJeCU
aXEKHVHFFMzLpA7E0VDi4K9Bqhg31Q1k/PEXd/lFhPKv1XFLSu5c+zy+Kmq4/eM+
BaotTD3jGeFvIY/ChkWfO42z7pTZaKRUiMCnjRdkZ/LmHTNB3PvdvSKkCnqH6xdX
Gxfk5h1XveqLndiGNrFDNuHfQRFLqFGOiGZZOKmJaoCu5Xn+3rGmHUUICcxxabI=
=RZgW
-----END PGP SIGNATURE-----