t7605: use test_path_is_file instead of test -f#2067
Open
MansiSingh17 wants to merge 1 commit intogitgitgadget:masterfrom
Open
t7605: use test_path_is_file instead of test -f#2067MansiSingh17 wants to merge 1 commit intogitgitgadget:masterfrom
MansiSingh17 wants to merge 1 commit intogitgitgadget:masterfrom
Conversation
Replace old-style 'test -f' path checks with the modern test_path_is_file helper in the merge_c1_to_c2_cmds block. The helper provides clearer failure messages and is the established convention in Git's test suite. These instances were found using: grep -rn "test -[efd]" t/ --include="*.sh" Signed-off-by: Mansi <mansimaanu8627@gmail.com>
Author
|
/submit |
|
Submitted as pull.2067.git.1773120813628.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): "Mansi Singh via GitGitGadget" <gitgitgadget@gmail.com> writes:
The e-mail header seems to imply you are "Mansi Singh". Do you want
to be known to this community under that name, or just "Mansi"?
> From: Mansi <mansimaanu8627@gmail.com>
>
> Replace old-style 'test -f' path checks with the modern
> test_path_is_file helper in the merge_c1_to_c2_cmds block.
>
> The helper provides clearer failure messages and is the
> established convention in Git's test suite.
OK.
> These instances were found using:
> grep -rn "test -[efd]" t/ --include="*.sh"
People seem to add the above paragraph to their test-path helper
patches, but unless the coverage of the work is fairly thorough and
you want to say "all the similar issues should be found with this
command and I addressed all of them", I do not see much point saying
how you found one of them and addressed it.
You could have used "git grep -e <pattern> -- t/\*.sh", or you could
have been working to fix something in t7605 and noticed these while
you were doing something else to the file.
I do not see it as too huge a deal and it is probably not a cause to
send in another iteration once it is already written, though.
> Signed-off-by: Mansi <mansimaanu8627@gmail.com>
No matter which name you pick, this should match the identity used
on your in-body "From:" header. In this message you are using the
same "Mansi" with address, which is good, but see also
Documentation/SubmittingPatches::real-name section.
> diff --git a/t/t7605-merge-resolve.sh b/t/t7605-merge-resolve.sh
> index 5d56c38546..44de97a480 100755
> --- a/t/t7605-merge-resolve.sh
> +++ b/t/t7605-merge-resolve.sh
> @@ -34,9 +34,9 @@ merge_c1_to_c2_cmds='
> test "$(git rev-parse c1)" = "$(git rev-parse HEAD^1)" &&
> test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" &&
> git diff --exit-code &&
> - test -f c0.c &&
> - test -f c1.c &&
> - test -f c2.c &&
> + test_path_is_file c0.c &&
> + test_path_is_file c1.c &&
> + test_path_is_file c2.c &&
The patch is quite straight-forward. Good.
> test 3 = $(git ls-tree -r HEAD | wc -l) &&
> test 3 = $(git ls-files | wc -l)
> '
>
> base-commit: d181b9354cf85b44455ce3ca9e6af0b9559e0ae2 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.