Re: [PATCH v2] reset: Better warning message on git reset --mixed <paths>

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ævar Arnfjörð Bjarmason  <avarab@xxxxxxxxx> writes:

> When you call "git reset --mixed <paths>" git will complain that using
> mixed with paths is deprecated:
>
>     warning: --mixed option is deprecated with paths.
>
> That doesn't tell the user why it's deprecated, or what he should use
> instead. Expand on the warning and tell the user to just omit --mixed:
>
>     warning: --mixed with paths is deprecated; use 'git reset -- <paths>' instead
>
> The exact wording of the warning was suggested by Jonathan Nieder.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
>
> On Sat, Aug 14, 2010 at 21:05, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
>> Maybe:
>>
>>  warning: --mixed with paths is deprecated; use 'git reset -- <paths>' instead
>
> That's better, thanks. Here's an amended version, and with tests this
> time.

While the new message is an improvement, I do not think the added test
that checks the exact wording is good.

Think for 5 seconds what the expected code change would be to break that
test.  I can only think of two realistic cases.  (1) Command line parsing
is broken and "reset --mixed <pathspec>" does not go through the codepath
that produces this warning anymore; (2) we deem that we had the
deprecation long enough and error out on this usage; or (3) Somebody comes
up with an even better wording but the string in this test still expects
suboptimal warning.

When we change this to an error, that is a behaviour change (it will
change the exit status as well), and it would be Ok to force the person
who does such a change to update the test.  But (3) shows that this test
is making it harder for people to improve the wording than necessary;
isn't it sufficient to check if any warning is issued at all?

I personally do not think this deserves to consume a new test number,
which is rather a scarce resource.

> diff --git a/t/t7112-reset-messages.sh b/t/t7112-reset-messages.sh
> new file mode 100755
> index 0000000..6f2669b
> --- /dev/null
> +++ b/t/t7112-reset-messages.sh
> @@ -0,0 +1,33 @@
> ...
> +test_expect_success 'git reset --mixed <paths> warning' '
> +	# Not test_commit() due to "ambiguous argument [..] both revision
> +	# and filename"

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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]