Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-08 Thread Philip Oakley

From: Jeff King p...@peff.net
Sent: Sunday, September 08, 2013 5:06 AM

Was there some objective argument made that I missed?


Here's more; human semantics:



Isn't this one of those pick any two from three tasks:
'human', 'objective', 'argument'.

Only 1/6 of the time is an 'objective' answer the result. 


Philip
Between thee and me there's nowt so queer as fowk,
and I ain't so sure about thee. old Yorkshire saying.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-08 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 A(ny) sanely defined compare A with B function should yield the
 result of subtracting B from A, i.e. cmp(A,B) should be like (A-B).
 That is what you feed qsort() and bsearch() (it is not limited to C;
 you see the same in sort { $a = $b }).  The definition naturally
 makes cmp(A,B)  0 like A  B and cmp(A,B)  0 like A  B.
 ---

 Ah, you mean if you think that the compare function should behave like
 C *_cmp functions, it should be A-B. Perhaps it is simply that I do not
 think of the function in those terms, but more like show me the
 differences from B to A.




  Otherwise why would so many
  existing test frameworks do it the other way?
 
 Which many existing frameworks do it the other way?

 John mentioned JUnit, NUnit, and PHPUnit earlier in the thread. I
 believe that Ruby's Test::Unit::Assertions also has
 assert_equal(expected, actual).

Especially the last one can be excused.  is A and B equal is a
binary between yes and no.  If A and B are equal, B and A are
equal, and it becomes more like which endianness is correct? as
you mentioned earlier.

I think the real cause of confusion is that cmp(1) is not a
comparison in that sense but is an equality check; test_cmp has a
dual purpose in that its primary use as did the previous step
produce correct result? is an equality check and the order does not
really matter, but its secondary purpose, to show how the actual
output deviated from the norm, has to be done by subtracting the
expected result from the actual result.

As I said, I am somewhat sympathetic to those who want to see such
subtraction spelled as cmp(Actual,Expect), but we are so used to the
order diff(1) takes expect and actual to do that subtraction in
that order, so using diff(Expect,Actual) order is not that wrong.

Calling the abstraction test_diff might have avoided the wasted
brain bandwidth in this thread, but I do not think renaming it in
test-lib-functions.sh is worth the trouble, either ;-)

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-08 Thread Felipe Contreras
On Sun, Sep 8, 2013 at 1:33 PM, Junio C Hamano gits...@pobox.com wrote:
 Jeff King p...@peff.net writes:

 Calling the abstraction test_diff might have avoided the wasted
 brain bandwidth in this thread, but I do not think renaming it in
 test-lib-functions.sh is worth the trouble, either ;-)

Yes, but then it wouldn't be clear what's the main purpose of
test_diff(). Primarily what we want is to check that they are the
same. The diff is secondary, and it's not actually *needed*.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-08 Thread Felipe Contreras
On Sun, Sep 8, 2013 at 12:02 AM, Jeff King p...@peff.net wrote:
 On Sat, Sep 07, 2013 at 11:52:10PM -0500, Felipe Contreras wrote:

  Ah, you mean if you think that the compare function should behave like
  C *_cmp functions, it should be A-B. Perhaps it is simply that I do not
  think of the function in those terms, but more like show me the
  differences from B to A.

 But that is the problem, you are unable to ignore the implementation.
 You don't see test_cmp(), you see 'diff -u'.

 Yes, I already said earlier in the thread:

   I certainly think of test_cmp A B as differences from A to B, and
   the order makes sense. IOW, the test_cmp is diff abstraction is
   leaky, and that is fine (if it were not leaky, then order would not
   matter at all, but it clearly does).

Then I don't think you can give a fair and objective opinion about
what should be the ideal ordering of the arguments. You would be
forever biased to whatever is the order of 'diff -u'.

 And I do not think it is a problem. The point of the function is not to
 abstract away the idea of comparison. The point is to give a hook for
 people on systems without diff -u to run the test suite.

The point according to whom? I say it's the other way around.

  John mentioned JUnit, NUnit, and PHPUnit earlier in the thread. I
  believe that Ruby's Test::Unit::Assertions also has
  assert_equal(expected, actual).

 That's because they all do first expect, then actual.

 assert_equal( expected, actual, failure_message = nil )
 assert_not_equal( expected, actual, failure_message = nil )

 That's why.

 I do not see any reason why not_equal would not also work as
 assert_not_equal(actual, expected). Maybe I am missing your point.

All right, maybe it's because Ruby started in Japan, and their
sentences have very different orderings, maybe it's legacy from
another test framework, maybe there's no reason at all and somebody
just randomly picked them.

Either way the fact that others are doing it differently doesn't mean
that's the best way, that would be argumentum ad populum, and mothers
are keen to remind us that if other kids are jumping the bridge, that
doesn't mean we should too.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-08 Thread Jeff King
On Sun, Sep 08, 2013 at 06:25:45PM -0500, Felipe Contreras wrote:

  And I do not think it is a problem. The point of the function is not to
  abstract away the idea of comparison. The point is to give a hook for
  people on systems without diff -u to run the test suite.
 
 The point according to whom? I say it's the other way around.

The point according to 82ebb0b (add test_cmp function for test scripts,
2008-03-12). I wish I had simply called it test_diff back then, and then
this conversation could have never occurred.

 Either way the fact that others are doing it differently doesn't mean
 that's the best way, that would be argumentum ad populum, and mothers
 are keen to remind us that if other kids are jumping the bridge, that
 doesn't mean we should too.

Did I once say my way of thinking about it is the best way? No. I said
I think it is a matter of preference. I mentioned other systems using
a particular ordering to show that the set of people who prefer it the
other way is non-zero.

Feel free to respond, but I have no interest in discussing this any
further with you. This thread has become a giant time sink, and I have
nothing else to say on the matter that I have not already said.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-08 Thread Felipe Contreras
On Sun, Sep 8, 2013 at 6:45 PM, Jeff King p...@peff.net wrote:
 On Sun, Sep 08, 2013 at 06:25:45PM -0500, Felipe Contreras wrote:

  And I do not think it is a problem. The point of the function is not to
  abstract away the idea of comparison. The point is to give a hook for
  people on systems without diff -u to run the test suite.

 The point according to whom? I say it's the other way around.

 The point according to 82ebb0b (add test_cmp function for test scripts,
 2008-03-12). I wish I had simply called it test_diff back then, and then
 this conversation could have never occurred.

So? That's the original intention from the author, it's not the point
of the function from the project's point of view, or what the point
_should_ be.

 Either way the fact that others are doing it differently doesn't mean
 that's the best way, that would be argumentum ad populum, and mothers
 are keen to remind us that if other kids are jumping the bridge, that
 doesn't mean we should too.

 Did I once say my way of thinking about it is the best way? No. I said
 I think it is a matter of preference. I mentioned other systems using
 a particular ordering to show that the set of people who prefer it the
 other way is non-zero.

That doesn't show it they prefer it that way. They could be in the
same situation than us, they might prefer it a different way, but they
are stuck with what they chose long time ago.

 Feel free to respond, but I have no interest in discussing this any
 further with you. This thread has become a giant time sink, and I have
 nothing else to say on the matter that I have not already said.

It's all right, I don't think you were contributing much more than;
diff -u does $expect $actual.

In case the above gets misconstrued as an insult, it's not meant that
way, Jeff said it himself; he cannot depart his view away from the
diff -u implementation.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-07 Thread Felipe Contreras
On Wed, Sep 4, 2013 at 1:36 PM, Jeff King p...@peff.net wrote:
 On Wed, Sep 04, 2013 at 10:38:03AM -0700, Junio C Hamano wrote:

  This is way off tangent, but I am somewhat sympathetic to Felipe's
  compare actual with expect, with reservations.
 
  This isn't an argument either way, but note that JUnit (and NUnit and
  PHPUnit) all have assertEquals methods that take the arguments in the
  order expect, actual.  I've always assumed that Git's test framework
  was imitating that,...

 No.  See 82ebb0b6 (add test_cmp function for test scripts,
 2008-03-12).  The test_cmp was a replacement for diff -u, and
 the same order we fed diff -u, i.e. expect then actual, was
 carried over.

 I don't think it was intentional at the time. But over the intervening 5
 years, I have noticed that I certainly think of test_cmp A B as
 differences from A to B, and the order makes sense. IOW, the test_cmp
 is diff abstraction is leaky, and that is fine (if it were not leaky,
 then order would not matter at all, but it clearly does).

 But let's take a step back. This seems like an endian-ness issue to me.
 I.e., some people prefer one order for test assertions, and other people
 prefer the other. Is anyone actually right, or is this simply a matter
 of preference? And if it is simply a matter of preference, then why
 bother going through the pain of changing the current project standard?

 Though I prefer the current, I can certainly live and adapt to a changed
 standard, and I do not mind doing so if there is a good reason. But I've
 yet to see any argument beyond it is not what I like. Which to me
 argues for the status quo as the path of least resistance.

Didn't Junio already provided reasoning?

Here's more; human semantics:

Computer, compare A with B
cmp(A, B)

Why would I write?

cmp(B, A)

Could you even construct an English sentence that starts with B, and then A?

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-07 Thread Jeff King
On Sat, Sep 07, 2013 at 10:11:49PM -0500, Felipe Contreras wrote:

  Though I prefer the current, I can certainly live and adapt to a changed
  standard, and I do not mind doing so if there is a good reason. But I've
  yet to see any argument beyond it is not what I like. Which to me
  argues for the status quo as the path of least resistance.
 
 Didn't Junio already provided reasoning?

If the reasoning is cmp(actual, expect) makes more sense to humans
then I do not think it is universal. Otherwise why would so many
existing test frameworks do it the other way? And that is why I said it
seems more like an issue of personal preference than a universal truth.

Was there some objective argument made that I missed?

 Here's more; human semantics:
 
 Computer, compare A with B
 cmp(A, B)
 
 Why would I write?
 
 cmp(B, A)
 
 Could you even construct an English sentence that starts with B, and then A?

Computer, given that we expect B, how does A differ?. Or Computer, we
expect B; does A match it?

Or any number of variations. I'm sure you will say but those seem
awkward and unlike how I think about it. But that was my point; it
seems to be a matter of preference.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-07 Thread Felipe Contreras
On Sat, Sep 7, 2013 at 11:06 PM, Jeff King p...@peff.net wrote:
 On Sat, Sep 07, 2013 at 10:11:49PM -0500, Felipe Contreras wrote:

  Though I prefer the current, I can certainly live and adapt to a changed
  standard, and I do not mind doing so if there is a good reason. But I've
  yet to see any argument beyond it is not what I like. Which to me
  argues for the status quo as the path of least resistance.

 Didn't Junio already provided reasoning?

 If the reasoning is cmp(actual, expect) makes more sense to humans
 then I do not think it is universal.

No.

---
A(ny) sanely defined compare A with B function should yield the
result of subtracting B from A, i.e. cmp(A,B) should be like (A-B).
That is what you feed qsort() and bsearch() (it is not limited to C;
you see the same in sort { $a = $b }).  The definition naturally
makes cmp(A,B)  0 like A  B and cmp(A,B)  0 like A  B.
---

 Otherwise why would so many
 existing test frameworks do it the other way?

Which many existing frameworks do it the other way?

 Here's more; human semantics:

 Computer, compare A with B
 cmp(A, B)

 Why would I write?

 cmp(B, A)

 Could you even construct an English sentence that starts with B, and then A?

 Computer, given that we expect B, how does A differ?. Or Computer, we
 expect B; does A match it?

 Or any number of variations. I'm sure you will say but those seem
 awkward and unlike how I think about it. But that was my point; it
 seems to be a matter of preference.

Really? You think any sane human being would prefer:

Computer, given that we expect B, how does A differ?

To:

Computer, compare A with B

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-07 Thread Felipe Contreras
On Sat, Sep 7, 2013 at 11:13 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 On Sat, Sep 7, 2013 at 11:06 PM, Jeff King p...@peff.net wrote:
 On Sat, Sep 07, 2013 at 10:11:49PM -0500, Felipe Contreras wrote:

  Though I prefer the current, I can certainly live and adapt to a changed
  standard, and I do not mind doing so if there is a good reason. But I've
  yet to see any argument beyond it is not what I like. Which to me
  argues for the status quo as the path of least resistance.

 Didn't Junio already provided reasoning?

 If the reasoning is cmp(actual, expect) makes more sense to humans
 then I do not think it is universal.

 No.

 ---
 A(ny) sanely defined compare A with B function should yield the
 result of subtracting B from A, i.e. cmp(A,B) should be like (A-B).
 That is what you feed qsort() and bsearch() (it is not limited to C;
 you see the same in sort { $a = $b }).  The definition naturally
 makes cmp(A,B)  0 like A  B and cmp(A,B)  0 like A  B.
 ---

 Otherwise why would so many
 existing test frameworks do it the other way?

 Which many existing frameworks do it the other way?

 Here's more; human semantics:

 Computer, compare A with B
 cmp(A, B)

 Why would I write?

 cmp(B, A)

 Could you even construct an English sentence that starts with B, and then A?

 Computer, given that we expect B, how does A differ?. Or Computer, we
 expect B; does A match it?

 Or any number of variations. I'm sure you will say but those seem
 awkward and unlike how I think about it. But that was my point; it
 seems to be a matter of preference.

 Really? You think any sane human being would prefer:

 Computer, given that we expect B, how does A differ?

And btw, that could barely be translated to cmp(B, A), probably cmp_given(B, A).

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-07 Thread Jeff King
On Sat, Sep 07, 2013 at 11:13:10PM -0500, Felipe Contreras wrote:

  If the reasoning is cmp(actual, expect) makes more sense to humans
  then I do not think it is universal.
 
 No.
 
 ---
 A(ny) sanely defined compare A with B function should yield the
 result of subtracting B from A, i.e. cmp(A,B) should be like (A-B).
 That is what you feed qsort() and bsearch() (it is not limited to C;
 you see the same in sort { $a = $b }).  The definition naturally
 makes cmp(A,B)  0 like A  B and cmp(A,B)  0 like A  B.
 ---

Ah, you mean if you think that the compare function should behave like
C *_cmp functions, it should be A-B. Perhaps it is simply that I do not
think of the function in those terms, but more like show me the
differences from B to A.

  Otherwise why would so many
  existing test frameworks do it the other way?
 
 Which many existing frameworks do it the other way?

John mentioned JUnit, NUnit, and PHPUnit earlier in the thread. I
believe that Ruby's Test::Unit::Assertions also has
assert_equal(expected, actual).

  Or any number of variations. I'm sure you will say but those seem
  awkward and unlike how I think about it. But that was my point; it
  seems to be a matter of preference.
 
 Really? You think any sane human being would prefer:
 
 Computer, given that we expect B, how does A differ?
 
 To:
 
 Computer, compare A with B

I already said that is how I think about it. If you want to call me not
sane, feel free. But I do not see that this line of discussion is going
anywhere productive.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-07 Thread Felipe Contreras
On Sat, Sep 7, 2013 at 11:26 PM, Jeff King p...@peff.net wrote:
 On Sat, Sep 07, 2013 at 11:13:10PM -0500, Felipe Contreras wrote:

  If the reasoning is cmp(actual, expect) makes more sense to humans
  then I do not think it is universal.

 No.

 ---
 A(ny) sanely defined compare A with B function should yield the
 result of subtracting B from A, i.e. cmp(A,B) should be like (A-B).
 That is what you feed qsort() and bsearch() (it is not limited to C;
 you see the same in sort { $a = $b }).  The definition naturally
 makes cmp(A,B)  0 like A  B and cmp(A,B)  0 like A  B.
 ---

 Ah, you mean if you think that the compare function should behave like
 C *_cmp functions, it should be A-B. Perhaps it is simply that I do not
 think of the function in those terms, but more like show me the
 differences from B to A.

But that is the problem, you are unable to ignore the implementation.
You don't see test_cmp(), you see 'diff -u'.

  Otherwise why would so many
  existing test frameworks do it the other way?

 Which many existing frameworks do it the other way?

 John mentioned JUnit, NUnit, and PHPUnit earlier in the thread. I
 believe that Ruby's Test::Unit::Assertions also has
 assert_equal(expected, actual).

That's because they all do first expect, then actual.

assert_equal( expected, actual, failure_message = nil )
assert_not_equal( expected, actual, failure_message = nil )

That's why.

  Or any number of variations. I'm sure you will say but those seem
  awkward and unlike how I think about it. But that was my point; it
  seems to be a matter of preference.

 Really? You think any sane human being would prefer:

 Computer, given that we expect B, how does A differ?

 To:

 Computer, compare A with B

 I already said that is how I think about it. If you want to call me notn
 sane, feel free. But I do not see that this line of discussion is going
 anywhere productive.

Again, that's because you are already thinking on the resulting diff,
based on the 'diff -u' command, but that's not in question here. Even
if test_cmp() didn't return an diff (it just ran cmp), it would be
useful, as ultimately we want to test for failures. Ultimately what we
want is to check that A is equal to B, so it's natural to tell the
computer compare A with B, and if you don't think so, then yeah, I
think you are insane.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-07 Thread Jeff King
On Sat, Sep 07, 2013 at 11:52:10PM -0500, Felipe Contreras wrote:

  Ah, you mean if you think that the compare function should behave like
  C *_cmp functions, it should be A-B. Perhaps it is simply that I do not
  think of the function in those terms, but more like show me the
  differences from B to A.
 
 But that is the problem, you are unable to ignore the implementation.
 You don't see test_cmp(), you see 'diff -u'.

Yes, I already said earlier in the thread:

  I certainly think of test_cmp A B as differences from A to B, and
  the order makes sense. IOW, the test_cmp is diff abstraction is
  leaky, and that is fine (if it were not leaky, then order would not
  matter at all, but it clearly does).

And I do not think it is a problem. The point of the function is not to
abstract away the idea of comparison. The point is to give a hook for
people on systems without diff -u to run the test suite.

  John mentioned JUnit, NUnit, and PHPUnit earlier in the thread. I
  believe that Ruby's Test::Unit::Assertions also has
  assert_equal(expected, actual).
 
 That's because they all do first expect, then actual.
 
 assert_equal( expected, actual, failure_message = nil )
 assert_not_equal( expected, actual, failure_message = nil )
 
 That's why.

I do not see any reason why not_equal would not also work as
assert_not_equal(actual, expected). Maybe I am missing your point.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-04 Thread John Keeping
On Wed, Sep 04, 2013 at 09:47:12AM -0700, Junio C Hamano wrote:
 Jonathan Nieder jrnie...@gmail.com writes:
 
  test_cmp_rev follows the same order of arguments a diff -u and
  produces the same output as plain git diff.  It's perfectly readable
  and normal.
 
 This is way off tangent, but I am somewhat sympathetic to Felipe's
 compare actual with expect, with reservations.

This isn't an argument either way, but note that JUnit (and NUnit and
PHPUnit) all have assertEquals methods that take the arguments in the
order expect, actual.  I've always assumed that Git's test framework
was imitating that, although I have no idea why that particular order is
chosen and is so common.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-04 Thread Junio C Hamano
John Keeping j...@keeping.me.uk writes:

 On Wed, Sep 04, 2013 at 09:47:12AM -0700, Junio C Hamano wrote:
 Jonathan Nieder jrnie...@gmail.com writes:
 
  test_cmp_rev follows the same order of arguments a diff -u and
  produces the same output as plain git diff.  It's perfectly readable
  and normal.
 
 This is way off tangent, but I am somewhat sympathetic to Felipe's
 compare actual with expect, with reservations.

 This isn't an argument either way, but note that JUnit (and NUnit and
 PHPUnit) all have assertEquals methods that take the arguments in the
 order expect, actual.  I've always assumed that Git's test framework
 was imitating that,...

No.  See 82ebb0b6 (add test_cmp function for test scripts,
2008-03-12).  The test_cmp was a replacement for diff -u, and
the same order we fed diff -u, i.e. expect then actual, was
carried over.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-04 Thread Jeff King
On Wed, Sep 04, 2013 at 10:38:03AM -0700, Junio C Hamano wrote:

  This is way off tangent, but I am somewhat sympathetic to Felipe's
  compare actual with expect, with reservations.
 
  This isn't an argument either way, but note that JUnit (and NUnit and
  PHPUnit) all have assertEquals methods that take the arguments in the
  order expect, actual.  I've always assumed that Git's test framework
  was imitating that,...
 
 No.  See 82ebb0b6 (add test_cmp function for test scripts,
 2008-03-12).  The test_cmp was a replacement for diff -u, and
 the same order we fed diff -u, i.e. expect then actual, was
 carried over.

I don't think it was intentional at the time. But over the intervening 5
years, I have noticed that I certainly think of test_cmp A B as
differences from A to B, and the order makes sense. IOW, the test_cmp
is diff abstraction is leaky, and that is fine (if it were not leaky,
then order would not matter at all, but it clearly does).

But let's take a step back. This seems like an endian-ness issue to me.
I.e., some people prefer one order for test assertions, and other people
prefer the other. Is anyone actually right, or is this simply a matter
of preference? And if it is simply a matter of preference, then why
bother going through the pain of changing the current project standard?

Though I prefer the current, I can certainly live and adapt to a changed
standard, and I do not mind doing so if there is a good reason. But I've
yet to see any argument beyond it is not what I like. Which to me
argues for the status quo as the path of least resistance.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-04 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Wed, Sep 04, 2013 at 10:38:03AM -0700, Junio C Hamano wrote:

  This is way off tangent, but I am somewhat sympathetic to Felipe's
  compare actual with expect, with reservations.
 
  This isn't an argument either way, but note that JUnit (and NUnit and
  PHPUnit) all have assertEquals methods that take the arguments in the
  order expect, actual.  I've always assumed that Git's test framework
  was imitating that,...
 
 No.  See 82ebb0b6 (add test_cmp function for test scripts,
 2008-03-12).  The test_cmp was a replacement for diff -u, and
 the same order we fed diff -u, i.e. expect then actual, was
 carried over.

 I don't think it was intentional at the time.

I do not think so, either. The only thing we cared about was are
they equal.  And from the point of view of test_cmp exit status,
that still the only thing we care about.  Comparison to see which is
greater is a superset of equality check we needed, and in that
context, the order did not and does not matter.

One only starts to notice the order of comparison when one starts
thinking about the comparison in terms of what is subtracted from
what, and at that point, one realizes that diff A B that gives us
what was lost from A to B as - and what was gained in B relative
to A as + is showing the result of subtracting A from B.  And that
subtraction is backwards from the cmp(A,B) that subtracts B from A,
which is the usual convention.

 Though I prefer the current, I can certainly live and adapt to a changed
 standard, and I do not mind doing so if there is a good reason. But I've
 yet to see any argument beyond it is not what I like. Which to me
 argues for the status quo as the path of least resistance.

As I think test_cmp is primarily about equality comparison, I do not
think it is worth switching and adapting.  Switching makes sense
only in my dreams where we did not have to worry about in-flight
topics and people's brains that are used to the current order.

That is exactly where my with reservations came from.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-03 Thread Jonathan Nieder
SZEDER Gábor wrote:
 On Tue, Sep 03, 2013 at 08:39:54AM -0500, Felipe Contreras wrote:

 There are two ways to fix an inconsistency, the other way is to fix
 test_cmp. But that would be a change, and change is not welcome in
 Git.

 It depends on the change, I suppose.  I agree, changing 3k+ lines just
 to avoid yoda conditions...  I doubt the gain worth the code churn.

Especially when the idiom being changed is not even being made better.
;-)

test_cmp_rev follows the same order of arguments a diff -u and
produces the same output as plain git diff.  It's perfectly readable
and normal.  I think Felipe is pushing buttons and testing boundaries.

But in the process came a report of a missing test_cmp_rev use, which
is useful.  Patch follows.

While at it, I rerolled the other patches from the series to clarify
their commit messages (replacing fix something with a fuller
description).

Improvements welcome, as always.  Thanks.

Felipe Contreras (3):
  rev-parse test: modernize quoting and whitespace
  rev-parse test: use test_must_fail, not if command; then false; fi
  rev-parse test: use standard test functions for setup

Jonathan Nieder (1):
  rev-parse test: use test_cmp instead of test builtin

 t/t6101-rev-parse-parents.sh | 110 +++
 1 file changed, 79 insertions(+), 31 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-03 Thread Jeff King
On Tue, Sep 03, 2013 at 10:04:19AM -0700, Jonathan Nieder wrote:

  It depends on the change, I suppose.  I agree, changing 3k+ lines just
  to avoid yoda conditions...  I doubt the gain worth the code churn.
 
 Especially when the idiom being changed is not even being made better.
 ;-)

Yes. IMHO it is not just not worth the churn but actively making the
code less readable.

 While at it, I rerolled the other patches from the series to clarify
 their commit messages (replacing fix something with a fuller
 description).

The series looks fine to me, modulo the fix up in v2 of 4/4.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions

2013-09-03 Thread Felipe Contreras
On Tue, Sep 3, 2013 at 12:04 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 SZEDER Gábor wrote:
 On Tue, Sep 03, 2013 at 08:39:54AM -0500, Felipe Contreras wrote:

 There are two ways to fix an inconsistency, the other way is to fix
 test_cmp. But that would be a change, and change is not welcome in
 Git.

 It depends on the change, I suppose.  I agree, changing 3k+ lines just
 to avoid yoda conditions...  I doubt the gain worth the code churn.

 Especially when the idiom being changed is not even being made better.
 ;-)

But it is better.

 test_cmp_rev follows the same order of arguments a diff -u and
 produces the same output as plain git diff.  It's perfectly readable
 and normal.  I think Felipe is pushing buttons and testing boundaries.

Those are irrelevant implementation details.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html