Re: [PATCH] reset: allow "-" short hand for previous commit

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

 



Sudhanshu Shekhar <sudshekhar02@xxxxxxxxx> writes:

>> What should worry us even more is what the user would get when @{-1}
>> does not resolve to something the command can use.  It would be bad
>> if we give an error message with @{-1} in it that the user never
>> typed (and may not even understand what it means).
>
> I apologize for having overlooked this use case. 

Thanking is fine, but there is no need to apologize in response to
review comments. We are imperfect humans and review exchanges are
designed to allow us cover points each of us missed in our
collective effort to make Git better.

> Another thing, can someone guide me regarding the right place to add a
> test case? Should it be t7102-reset.sh or some other file?

At the end of that file sounds like a reasonable choice.  You would
want to test various cases, such as (1) what happens when there is
no @{-1} at all (you would need a newly initialied test repository,
just like the last test in that file creates mixed_worktree
repository for its own use), (2) what happens when there is, split
into two , i.e. (1-a) @{-1} does not exist but there is a file whose
name is "-"; (1-b) @{-1} does not exist and there is no file whose
name is "-"; (2-a) @{-1} exists but there is a file whose name is
"-"; and (2-b) @{-1} exists and there is no file whose name is "-".

Do not just test (2-b) and declare victory---making sure that a
feature does not trigger when it should not is also important.

Thanks.
--
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]