Re: [PATCH] t1300: document some aesthetic failures of the config editor

[Jeff King <peff@peff.net>]

On Wed, Mar 28, 2018 at 06:33:55PM +0200, Johannes Schindelin wrote:

> On Fri, 29 Mar 2013, Jeff King wrote:
> 
> > Subject: [PATCH] t1300: document some aesthetic failures of the config editor

This is an old one. :) I had to go look up the old thread to refresh
myself.

> [...]
> Obviously, your example gives the impression that `git config --unset
> core.key` shoud *delete* that comment (that obviously is intended to
> document the section, not the `key` value).
> 
> And this is bad, really bad. And this comment does not make it better:
> 
> 	I think we may not attain that ideal without some natural language
> 	processing of the comments. But hey, no reason not to shoot for the
> 	stars. :)
> 
> There *is* a reason, a very good reason *not* to shoot for the stars.

I think you are reading more into my comment than was intended. No, I
don't think we plan to implement a sufficiently advanced AI to cover all
these cases. But as I said in the thread:

  It makes sense to me to document both via tests; even if we end up
  tweaking the expected behavior when the fix is actually implemented,
  the presence of the test still serves as a reminder of the issue.

So it was always intended for this test to give a general sense of the
problem, from which somebody interested could dig further and work on
it.

Probably the commit message could have made this more clear (or even an
in-code comment).

> Think about it. The `test_expect_failure` function is intended to
> demonstrate bugs, and once those bugs are fixed, the _failure should be
> turned into _success. And if somebody looks for work, they can look for
> test_expect_failure and find tons of micro-projects.
> 
> What you did there was to change some valid demonstration of a bug that
> can be fixed to something that cannot be fixed. So if an occasional lurker
> comes along, sees what you expect to be fixed, they would have said
> "Whoa!" and you lost a contribution.

Hypothetically, you may be right. But don't all bugs have some element
of this? People can find an expect_failure as a starting point, but
they'll have to dig into the background and history of the bug if they
want to know the subtleties. This one is just more subtle than some
others.

> On a positive note: I just finished work on a set of patches addressing
> this:
> https://github.com/git/git/compare/master...dscho:empty-config-section (I
> plan on submitting this tomorrow)

Great. If your series throws away my test and replaces it with something
more attainable (preferably with expect_success ;) ), I think that is
certainly a positive change.

-Peff