Thread (6 messages) 6 messages, 2 authors, 2020-01-16

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help