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 >
