Code review comment for lp:~fgallaire/bzr/fix-gmtime-lite

Revision history for this message
Vincent Ladeuil (vila) wrote :

Good work thanks !

I'm (pleasantly) surprised that this ends up being so lite (and focused).

As a side effect, it shows that bzr doesn't care about dates (which can't be guaranteed to be accurate in a distributed environment where host clocks can't be trusted).

grepping for 'gmtime(' I found a few references you probably want to fix too:

./bzrlib/doc_generate/ tt = time.gmtime(t)
./bzrlib/doc_generate/ tt = time.gmtime(t)
./bzrlib/doc_generate/ tt = time.gmtime(t)
./bzrlib/ date_string = time.strftime('%Y-%m-%dT%H:%M', time.gmtime())

(I don't think they matter as much but being consistent and using osutils.gmtime() instead of time.gmtime() remove confusion about why the code base would use two different versions).

This bug fix is worth mentioning in the news in doc/en/release-notes/bzr-2.8.txt

Final administrativia: please sign the CLA

review: Needs Fixing

« Back to merge proposal