Merge lp:~malept/bzr-builder/bug.394117 into lp:~james-w/bzr-builder/trunk-old

[Mark Lee]

[Mark Lee (malept)]

See bug #394117 for details.

[James Westby (james-w)]

> See bug #394117 for details.

Hi,

Thanks for looking on this, and your fix makes sense.

I just wonder if the code itself (rather than just the test)
should use the UTC timestamp. I don't think users would expect
that, but I wonder if we should to avoid issues with timezone
changes.

I think this is good to land, but if you have an opinion on
the above I would love to hear it.

Thanks,

James

review: Needs Information

[Mark Lee (malept)]

Hi,

> > See bug #394117 for details.
>
> Hi,
>
> Thanks for looking on this, and your fix makes sense.
>
> I just wonder if the code itself (rather than just the test)
> should use the UTC timestamp. I don't think users would expect
> that, but I wonder if we should to avoid issues with timezone
> changes.
>
> I think this is good to land, but if you have an opinion on
> the above I would love to hear it.

I think that if it's expected to be used by two or more packagers who are in different timezones (probably a common use case when PPA support is added), then UTC timestamps are the best solution. It shouldn't be too confusing if it's documented in `bzr help builder`.

-Mark

review: Abstain

[James Westby (james-w)]

Approved, thanks.

I'm also going to make the code itself use UTC.

Thanks,

James

review: Approve

[James Westby (james-w)]

And what do you know, once I look I realise the code
was using UTC all along.

Sorry for the delay and thanks for the contribution.

James

=== modified file 'tests/test_recipe.py'
--- tests/test_recipe.py	2009-04-22 18:44:03 +0000
+++ tests/test_recipe.py	2009-07-01 04:09:36 +0000
@@ -739,12 +739,12 @@
         self.assertTrue(rbranch1.different_shape_to(rbranch2))
 
     def test_substitute_time(self):
-        time = datetime.datetime.fromtimestamp(1)
+        time = datetime.datetime.utcfromtimestamp(1)
         base_branch = BaseRecipeBranch("base_url", "1-{time}")
         base_branch.substitute_time(time)
-        self.assertEqual("1-197001010100", base_branch.deb_version)
+        self.assertEqual("1-197001010000", base_branch.deb_version)
         base_branch.substitute_time(time)
-        self.assertEqual("1-197001010100", base_branch.deb_version)
+        self.assertEqual("1-197001010000", base_branch.deb_version)
 
     def test_substitute_revno(self):
         base_branch = BaseRecipeBranch("base_url", "1")