On Thursday 16 December 2010, Ralf Wildenhues wrote:
> * Stefano Lattarini wrote on Thu, Dec 16, 2010 at 01:27:10AM CET:
> > On Wednesday 15 December 2010, Ralf Wildenhues wrote:
> [ magic strings ]
> > > Why all the variation in these? That makes the tests harder to read.
> > >
> > I'd rather not change the use of magic strings on the other remake
> > tests, unless you insist. The test remak3a.test is IMHO simple enough
> > not to cause confusion, and changing the tests remake8{a,b}.test would
> > be quite cumbersome.
>
> I don't insist.
>
Thanks.
> > > > + test x'$(am_fingerprint)' = x'DummyValue'
> > >
> > > Why do you quote DummyValue here?
> > >
> > Partly for symmetry with the left side, and partly (especially I'd say)
> > to make the leading `x' stick out better as "not a part of the expected
> > value".
> >
> > > The leading x should not be needed on either side.
> > >
> > True, "should not" -- but I got in the habit of always using it, even
> > where technically not needed, rather then risking to forgot it one time
> > when it's really required.
> >
> > > (several instances below)
> > >
> > Should I remove them? (sorry if I ask, but the "should" above does
> > not make it clear if you're asking to remove them or just noting that
> > they are not really required).
>
> Ah, the fun of English negation and passive voice. I'm not totally firm
> in this area either, but I think that "should not be needed" has the
> meaning of "is not needed, at least I think so" rather than "don't do
> this or I'll poke you with a stick".
>
I think the same way too, but since you wrote "ok with nits addressed"
in the beginning of your mail, I wasn't sure whether that "should not"
were pointing out a nit (hence to be addressed).
> I hope somebody corrects me on this if my interpretation is wrong.
> > Attached is also the amended patch. I will push in 72 hours if there
> > are no more objections (and if all my testing on Linux and Solaris
> > succeeds).
>
> I don't object, but TBH, I didn't look at the patch again. ;-)
>
Thast shouldn't be really necessary if you've looked at my inline
comments.
> Thanks!
> Ralf
>
Thanks for the approval. I've now merged the patch into master,
and pushed.
Regards,
Stefano
