| Submitter | Mads Kiilerich |
|---|---|
| Date | Feb. 26, 2013, 2:39 a.m. |
| Message ID | <b1f0fd26bda704357708.1361846389@mk-desktop> |
| Download | mbox | patch |
| Permalink | /patch/1047/ |
| State | Accepted |
| Commit | a5d33446e46c11737c59106ba91fba88deabf3c3 |
| Headers | show |
Comments
On 02/26/2013 06:14 AM, Kevin Bullock wrote: > On 25 Feb 2013, at 8:39 PM, Mads Kiilerich wrote: > >> # HG changeset patch >> # User Mads Kiilerich <madski@unity3d.com> >> # Date 1361844560 -3600 >> # Branch stable >> # Node ID b1f0fd26bda704357708c73fdeb78f1b6f372238 >> # Parent 61c8327ced503bf7a1996337af711e0f4a58d4c0 >> templatefilters: add missing import of _ >> >> diff --git a/mercurial/templatefilters.py b/mercurial/templatefilters.py >> --- a/mercurial/templatefilters.py >> +++ b/mercurial/templatefilters.py >> @@ -5,6 +5,7 @@ >> # This software may be used and distributed according to the terms of the >> # GNU General Public License version 2 or any later version. >> >> +from i18n import _ >> import cgi, re, os, time, urllib >> import encoding, node, util, error >> import hbisect >> diff --git a/tests/test-template-engine.t b/tests/test-template-engine.t >> --- a/tests/test-template-engine.t >> +++ b/tests/test-template-engine.t >> @@ -44,4 +44,15 @@ >> 0 97e5f848f0936960273bbf75be6388cd0350a32b -1 0000000000000000000000000000000000000000 >> -1 0000000000000000000000000000000000000000 -1 0000000000000000000000000000000000000000 >> >> + $ hg tip --template '{fill(node, 7)}\n' >> + hg: parse error: fill expects an integer width >> + [255] >> + $ hg tip --template '{fill(node, "7")}\n' >> + ef4a3af >> + ceb5370 >> + e8500df >> + 6f7b191 >> + e48998e >> + 059af >> + >> $ cd .. > I'm confused what this test has to do with the code change. The test is testing the code change. The original code was added without test coverage (and without documentation). The new tests gives _some_ test coverage of this code so this exact problem won't happen again. It is very surprising that the fill width can't be specified as an integer but has to be a string (especially given the error message), so it would be very misleading just to add a test that showed that now it knew how to fail. /Mads
On 02/26/2013 05:38 PM, Kevin Bullock wrote: > On Feb 26, 2013, at 5:25 AM, Mads Kiilerich wrote: > >> On 02/26/2013 06:14 AM, Kevin Bullock wrote: >>> On 25 Feb 2013, at 8:39 PM, Mads Kiilerich wrote: >>> >>>> # HG changeset patch >>>> # User Mads Kiilerich <madski@unity3d.com> >>>> # Date 1361844560 -3600 >>>> # Branch stable >>>> # Node ID b1f0fd26bda704357708c73fdeb78f1b6f372238 >>>> # Parent 61c8327ced503bf7a1996337af711e0f4a58d4c0 >>>> templatefilters: add missing import of _ >>>> >>>> diff --git a/mercurial/templatefilters.py b/mercurial/templatefilters.py >>>> --- a/mercurial/templatefilters.py >>>> +++ b/mercurial/templatefilters.py >>>> @@ -5,6 +5,7 @@ >>>> # This software may be used and distributed according to the terms of the >>>> # GNU General Public License version 2 or any later version. >>>> >>>> +from i18n import _ >>>> import cgi, re, os, time, urllib >>>> import encoding, node, util, error >>>> import hbisect >>>> diff --git a/tests/test-template-engine.t b/tests/test-template-engine.t >>>> --- a/tests/test-template-engine.t >>>> +++ b/tests/test-template-engine.t >>>> @@ -44,4 +44,15 @@ >>>> 0 97e5f848f0936960273bbf75be6388cd0350a32b -1 0000000000000000000000000000000000000000 >>>> -1 0000000000000000000000000000000000000000 -1 0000000000000000000000000000000000000000 >>>> >>>> + $ hg tip --template '{fill(node, 7)}\n' >>>> + hg: parse error: fill expects an integer width >>>> + [255] >>>> + $ hg tip --template '{fill(node, "7")}\n' >>>> + ef4a3af >>>> + ceb5370 >>>> + e8500df >>>> + 6f7b191 >>>> + e48998e >>>> + 059af >>>> + >>>> $ cd .. >>> I'm confused what this test has to do with the code change. >> The test is testing the code change. >> >> The original code was added without test coverage (and without documentation). > Eh? How does adding a missing import of _ fix the fill() function? (This should be in the commit description.) It is really so trivial that it doesn't deserve a description. When an import is added because it was missing then it is obviously because some code uses it. When a test is added then it is obviously because it exercises the code path that failed without the fix and proves that it no longer fails. Please, let's get the proportions right and save the long explanations to stuff that really needs an explanation. /Mads
On 02/26/2013 03:39 AM, Mads Kiilerich wrote: > # HG changeset patch > # User Mads Kiilerich <madski@unity3d.com> > # Date 1361844560 -3600 > # Branch stable > # Node ID b1f0fd26bda704357708c73fdeb78f1b6f372238 > # Parent 61c8327ced503bf7a1996337af711e0f4a58d4c0 > templatefilters: add missing import of _ fix pushed to crew stable /Mads
Patch
diff --git a/mercurial/templatefilters.py b/mercurial/templatefilters.py --- a/mercurial/templatefilters.py +++ b/mercurial/templatefilters.py @@ -5,6 +5,7 @@ # This software may be used and distributed according to the terms of the # GNU General Public License version 2 or any later version. +from i18n import _ import cgi, re, os, time, urllib import encoding, node, util, error import hbisect diff --git a/tests/test-template-engine.t b/tests/test-template-engine.t --- a/tests/test-template-engine.t +++ b/tests/test-template-engine.t @@ -44,4 +44,15 @@ 0 97e5f848f0936960273bbf75be6388cd0350a32b -1 0000000000000000000000000000000000000000 -1 0000000000000000000000000000000000000000 -1 0000000000000000000000000000000000000000 + $ hg tip --template '{fill(node, 7)}\n' + hg: parse error: fill expects an integer width + [255] + $ hg tip --template '{fill(node, "7")}\n' + ef4a3af + ceb5370 + e8500df + 6f7b191 + e48998e + 059af + $ cd ..