Re: [PATCH 0/3] t6025: updating tests
From: Johannes Schindelin <hidden>
Date: 2020-01-16 21:46:43
Hi, On Fri, 17 Jan 2020, Shourya Shukla wrote:
Greetings everyone! This is my first ever contribution in the Open-Source Community and I chose Git for this purpose as I have been using this important tool to maintain my projects regularly. In this patch, I have: - modernized these tests so that they meet current Git CodingGuidlines[1] - replaced the pipe operator with the redirection operator so that one can detect the errors easily and precisely - used helper function 'test_path_is_file()' to replace 'test -f' checks in in the program as it improves the readability of the code and provides better error messages Also, I have some questions to better my understanding of the code: - In the statement, > git hash-object -t blob -w --stdin is it necessary to explicitly specify the type 'blob' of the hash-object? I have this question because it is the default type of hash-object.
It is the default type, but: 1) the code is not broken, so why fix it? 2) it _might_ be possible that the default changes, or can be configured in the future. The original author might just have wanted to stay safe.
- In the statement, > l=$(printf file | git hash-object -t blob -w --stdin) I have not used the redirection operator as this sub-shell will be executed separately, hence its error cannot be captured therefore the presence of '>' will not matter. Will using '>' improve the code?
It will be enhanced, though: printf file >file && l=$(git hash-object -t blob -w --stdin) will have a non-zero exit code if the `hash_object` call fails.
Thanks, Shourya Shukla Shourya Shukla (3): t6025: modernize style t6025: replace pipe with redirection operator t6025: use helpers to replace test -f <path>
Apart from one little issue in the first patch, this all looks very good to me. Thanks, Johannes
t/t6025-merge-symlinks.sh | 97 ++++++++++++++++++++------------------- 1 file changed, 50 insertions(+), 47 deletions(-) -- 2.20.1