[PATCH v5] Allow commit date before Unix epoch, clean fix for (issue2513)

Adrian Buehlmann adrian at cadifra.com
Mon Apr 4 03:07:37 EDT 2016


On 2016-04-04 03:13, Florent Gallaire wrote:
> 2016-04-04 1:45 GMT+02:00 Pierre-Yves David <pierre-yves.david at ens-lyon.org>:
>> Can we have a small inline comment that explain where this 0x7fffffff value
>> comes from?
> 
> This is already done in the parsedate() function in the same util.py file.
> 
>> As I said earlier, we want stronger testing of this (or pointer that this is
>> already tested properly testing in another patch),
>>
>> You are adding (okay, re-introducing) a space of possible date, we should
>> add automated testing for values in this space including possible limits.
> 
> Add all tests you want.

Reviewers usually don't do the work for contributers (they are busy
enough already). It should be pretty easy for you to add a couple of
testcases to existing tests. This is often done at the end of an
existing testsfile or wherever you see fit.

It's ok if you run the testsuite on Linux only, so it should be pretty
easy. See https://www.mercurial-scm.org/wiki/WritingTests

Perhaps, you can also add a few doctests to the datestr function. See
for example the pre-existing doctests for checkwinfilename() on line
1069 in util.py and
https://www.mercurial-scm.org/wiki/WritingTests#Writing_a_Python_doctest

>> If we don't do this, I'll have a low confidence that this patch is not
>> breaking windows again. And equally important, someone may as well silently
>> break that feature in the future, reintroducing regression.
> 
> You can be confident, this patch doesn't break Windows (more than it
> already is of course).

Having it tested in the testsuite is better.


More information about the Mercurial-devel mailing list