juju-release-tools

Code review comment for lp:~abentley/juju-release-tools/agents-stanzas-2

agents-stanzas-2
Merge into trunk

Revision history for this message

Aaron Bentley (abentley) wrote on 2015-11-10:

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

I've pushed a new version with the changes you asked for.

>> + self.version_name = date.today().strftime('%Y%m%d')
>
> Could this get weird if we build on machines in different
> timezones? maybe datetime.utcnow().strftime('%Y%m%d') is safer

The odds are low, because versions are only relevant if there are
competing item_names for a given content_id, and we don't do that.

Even if we did, items with the a given item_name will typically be built
on the same machine every time, so they will have the same timezone.

Still, there's no harm in using UTC time. Done.

>> + self.filename = filename + + @classmethod + def
>> for_ubuntu(cls, release, series, arch, version, revision_build,
>
> As an aside, I am constantly annoyed that debian calls the
> series/codename release, so when I work with lxc templates or
> scripts that make changelogs, I pass the series as the release.
> What we call a release is closer to debian version. While we don't
> see it often, I see that precise and trusty in the clouds are
> actually 12.04.5 and 14.04.2. There is no right ways to think about
> this.

The vagueness about release/series/codename bugs me, too. (And I can't
help wondering whether we really need both :-) Still, I tried to use
the terminology of the job. That meant that I had to do to do
"stanza['release'] = series" in make_stanzas(). Ugh.

> We need a single definition of this list that can be imported. Can
> you make this list a module level constant? I want formal
> definition of the series we support.

Done.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJWQgD5AAoJEK84cMOcf+9hAQQH/0+zqHNBoQ6+J4EwYMWcUln4
M0JaHGfe2XJXlrmlwqGRpYBtTrEcH9DWiCih1/2I7QyY4dXcpVoUtqkPqA8ROBTD
gZNjJRVQapWSStxs+4snfDVatGxmaM+/A+ayq4Vzp8XGSiteSxOQZVQRs0b3d+cE
3HooPlNx2m2pdqiY4s+6fDFkhqUSjJmyxq4un03LAmH/gjOhuZcyNzpt9+PjBfjI
6iI/xtC2U/hBW3gcZW8Qk7HyVCHPOW6aGKomMGNN9HX/E0vkZAfOihEdBc7xZYp2
H4hdA+s6UjzO9QjNDF37t7STmzLSXTYQeMmUF92rr9+yQed1leuU7BL6naPBkfQ=
=8nzG
-----END PGP SIGNATURE-----

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

I've pushed a new version with the changes you asked for.

>> +        self.version_name = date.today().strftime('%Y%m%d')
> 
> Could this get weird if we build on machines in different
> timezones? maybe datetime.utcnow().strftime('%Y%m%d') is safer

The odds are low, because versions are only relevant if there are
competing item_names for a given content_id, and we don't do that.

Even if we did, items with the a given item_name will typically be built
on the same machine every time, so they will have the same timezone.

Still, there's no harm in using UTC time.  Done.

>> +        self.filename = filename + +    @classmethod +    def
>> for_ubuntu(cls, release, series, arch, version, revision_build,
> 
> As an aside, I am constantly annoyed that debian calls the
> series/codename release, so when I work with lxc templates or
> scripts that make changelogs, I pass the series as the release.
> What we call a release is closer to debian version. While we don't
> see it often, I see that precise and trusty in the clouds are
> actually 12.04.5 and 14.04.2. There is no right ways to think about
> this.

The vagueness about release/series/codename bugs me, too.  (And I can't
help wondering whether we really need both :-)  Still, I tried to use
the terminology of the job.  That meant that I had to do to do
"stanza['release'] = series" in make_stanzas().  Ugh.

> We need a single definition of this list that can be imported. Can
> you make this list a module level constant? I want formal
> definition of the series we support.

Done.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

« Back to merge proposal