{"thread":{"id":"48152","subject":"[PATCH] test_must_be_empty: simplify file existence check","startedAt":"2018-03-26T11:58:32Z","lastAt":"2018-03-28T05:20:12Z","messageCount":6,"participants":["SZEDER Gábor","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"342990","messageId":"20180326115815.10180-1-szeder.dev@gmail.com","threadId":"48152","inReplyTo":null,"subject":"[PATCH] test_must_be_empty: simplify file existence check","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-03-26T11:58:15Z","receivedAt":"2018-03-26T11:58:32Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Commit ea3c87d0b7 (test_must_be_empty: make sure the file exists, not\njust empty, 2018-02-27) basically duplicated the 'test_path_is_file'\nhelper function in 'test_must_be_empty'.\n\nJust call 'test_path_is_file' to avoid this code duplication.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/test-lib-functions.sh | 7 ++-----\n 1 file changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex d2eaf5ab67..36ad8accdd 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -718,11 +718,8 @@ verbose () {\n # otherwise.\n \n test_must_be_empty () {\n-\tif ! test -f \"$1\"\n-\tthen\n-\t\techo \"'$1' is missing\"\n-\t\treturn 1\n-\telif test -s \"$1\"\n+\ttest_path_is_file \"$1\" &&\n+\tif test -s \"$1\"\n \tthen\n \t\techo \"'$1' is not empty, it contains:\"\n \t\tcat \"$1\"\n-- \n2.17.0.rc1.145.gfac1bf9500\n\n"},{"id":"342996","messageId":"20180326124847.GA21993@sigill.intra.peff.net","threadId":"48152","inReplyTo":"20180326115815.10180-1-szeder.dev@gmail.com","subject":"Re: [PATCH] test_must_be_empty: simplify file existence check","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-26T12:48:48Z","receivedAt":"2018-03-26T12:48:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 26, 2018 at 01:58:15PM +0200, SZEDER Gábor wrote:\n\n> Commit ea3c87d0b7 (test_must_be_empty: make sure the file exists, not\n> just empty, 2018-02-27) basically duplicated the 'test_path_is_file'\n> helper function in 'test_must_be_empty'.\n> \n> Just call 'test_path_is_file' to avoid this code duplication.\n\nThis looks like an improvement to me.\n\nI think you meant to reference 11395a3b4b16d9fc637ca2e41a6892ea2e6289ce,\nthough (I don't have ea3c87d0b7 at all).\n\n-Peff\n"},{"id":"343001","messageId":"CAM0VKjmCNDAKF=ueDBtscTUMJzVEZoFDA_CO6Oz9sTLUpyyiuQ@mail.gmail.com","threadId":"48152","inReplyTo":"20180326124847.GA21993@sigill.intra.peff.net","subject":"Re: [PATCH] test_must_be_empty: simplify file existence check","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-03-26T13:09:13Z","receivedAt":"2018-03-26T13:09:22Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Mar 26, 2018 at 2:48 PM, Jeff King <peff@peff.net> wrote:\n> On Mon, Mar 26, 2018 at 01:58:15PM +0200, SZEDER Gábor wrote:\n>\n>> Commit ea3c87d0b7 (test_must_be_empty: make sure the file exists, not\n>> just empty, 2018-02-27) basically duplicated the 'test_path_is_file'\n>> helper function in 'test_must_be_empty'.\n>>\n>> Just call 'test_path_is_file' to avoid this code duplication.\n>\n> This looks like an improvement to me.\n>\n> I think you meant to reference 11395a3b4b16d9fc637ca2e41a6892ea2e6289ce,\n> though (I don't have ea3c87d0b7 at all).\n\nIndeed.  I remember picking up that patch from the mailing list, which\nthen became ea3c87d0b7 in my repo.  What I don't remember is how I\nmanaged to build on top of this hand-picked commit instead of what has\nbeen in git.git for a month...\n"},{"id":"343002","messageId":"20180326131124.13003-1-szeder.dev@gmail.com","threadId":"48152","inReplyTo":"20180326124847.GA21993@sigill.intra.peff.net","subject":"[PATCH v2] test_must_be_empty: simplify file existence check","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-03-26T13:11:24Z","receivedAt":"2018-03-26T13:11:32Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Commit 11395a3b4b (test_must_be_empty: make sure the file exists, not\njust empty, 2018-02-27) basically duplicated the 'test_path_is_file'\nhelper function in 'test_must_be_empty'.\n\nJust call 'test_path_is_file' to avoid this code duplication.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n\nThe only change is to refer to the right commit in the log message.\n\n t/test-lib-functions.sh | 7 ++-----\n 1 file changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex d2eaf5ab67..36ad8accdd 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -718,11 +718,8 @@ verbose () {\n # otherwise.\n \n test_must_be_empty () {\n-\tif ! test -f \"$1\"\n-\tthen\n-\t\techo \"'$1' is missing\"\n-\t\treturn 1\n-\telif test -s \"$1\"\n+\ttest_path_is_file \"$1\" &&\n+\tif test -s \"$1\"\n \tthen\n \t\techo \"'$1' is not empty, it contains:\"\n \t\tcat \"$1\"\n-- \n2.17.0.rc1.148.gc92dc354c5\n\n"},{"id":"343193","messageId":"xmqqd0zp9i0d.fsf@gitster-ct.c.googlers.com","threadId":"48152","inReplyTo":"20180326131124.13003-1-szeder.dev@gmail.com","subject":"Re: [PATCH v2] test_must_be_empty: simplify file existence check","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-28T00:10:58Z","receivedAt":"2018-03-28T00:11:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder.dev@gmail.com> writes:\n\n> Commit 11395a3b4b (test_must_be_empty: make sure the file exists, not\n> just empty, 2018-02-27) basically duplicated the 'test_path_is_file'\n> helper function in 'test_must_be_empty'.\n>\n> Just call 'test_path_is_file' to avoid this code duplication.\n>\n> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n> ---\n>\n> The only change is to refer to the right commit in the log message.\n>\n>  t/test-lib-functions.sh | 7 ++-----\n>  1 file changed, 2 insertions(+), 5 deletions(-)\n>\n> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> index d2eaf5ab67..36ad8accdd 100644\n> --- a/t/test-lib-functions.sh\n> +++ b/t/test-lib-functions.sh\n> @@ -718,11 +718,8 @@ verbose () {\n>  # otherwise.\n>  \n>  test_must_be_empty () {\n> -\tif ! test -f \"$1\"\n> -\tthen\n> -\t\techo \"'$1' is missing\"\n> -\t\treturn 1\n> -\telif test -s \"$1\"\n> +\ttest_path_is_file \"$1\" &&\n> +\tif test -s \"$1\"\n>  \tthen\n>  \t\techo \"'$1' is not empty, it contains:\"\n>  \t\tcat \"$1\"\n\n\"Just call it\" is fine as an idea but\n\n\tA &&\n\tif B\n\tthen\n\t\t...\n\tfi\n\nis somewhat questionable.  Shouldn't we make it\n\n\tif A && B\n\tthen\n\t\t...\n\tfi\n\ninstead?  That way, if we ever need to add an else clause, the logic\nflow would be more obvious, no?\n\n"},{"id":"343206","messageId":"xmqqzi2s93p6.fsf@gitster-ct.c.googlers.com","threadId":"48152","inReplyTo":"xmqqd0zp9i0d.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2] test_must_be_empty: simplify file existence check","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-28T05:20:05Z","receivedAt":"2018-03-28T05:20:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>>  test_must_be_empty () {\n>> -\tif ! test -f \"$1\"\n>> -\tthen\n>> -\t\techo \"'$1' is missing\"\n>> -\t\treturn 1\n>> -\telif test -s \"$1\"\n>> +\ttest_path_is_file \"$1\" &&\n>> +\tif test -s \"$1\"\n>>  \tthen\n>>  \t\techo \"'$1' is not empty, it contains:\"\n>>  \t\tcat \"$1\"\n>\n> \"Just call it\" is fine as an idea but\n>\n> \tA &&\n> \tif B\n> \tthen\n> \t\t...\n> \tfi\n>\n> is somewhat questionable.  Shouldn't we make it\n>\n> \tif A && B\n> \tthen\n> \t\t...\n> \tfi\n>\n> instead?\n\nNah, you want to treat A's success as a condition *not* to enter the\n\"then\" clause in this case, so my rewrite is bogus.  SOrry for the\nnoise.\n\n\n\n"}]}