On 15/04/15 04:47, Lex Spoon wrote:
From 9cc607667a20317c837afd90d50c078da659b72f Mon Sep 17 00:00:00 2001 From: Lex Spoon <l...@lexspoon.org> Date: Sat, 11 Apr 2015 10:01:15 -0400 Subject: [PATCH] git-p4: Use -m when running p4 changes
This patch didn't want to apply for me, I'm not quite sure why but possibly it's become scrambled? Either that or I'm doing it wrong! If you use git send-email it should Just Work.
As an aside could you post reworked versions of patches with a subject line of [PATCH v2], [PATCH v3], etc, so reviewers can keep track of what's going on?
Note to other reviewers: the existing git-p4 has a --max-changes option for 'sync', but this doesn't do the same thing at all. It doesn't limit the number of changes requested from the server, it just limits the number of changes pulled down, after the p4 server has supplied those changes. This confused me at first!
Lex - I should have mentioned this before, but would you be able to add some documentation to Documentation/git-p4.txt to explain what your new option does? It would help to distinguish between your option and the existing --max-changes option.
I've put a few remarks below in your shell script; there are a few minor issues that could do with being tidied up.
Thanks! Luke <snip>
diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh new file mode 100755 index 0000000..73e545d --- /dev/null +++ b/t/t9818-git-p4-block.sh @@ -0,0 +1,64 @@ +#!/bin/sh + +test_description='git p4 fetching changes in multiple blocks' + +. ./lib-git-p4.sh + +test_expect_success 'start p4d' ' + start_p4d +' + +test_expect_success 'Create a repo with 100 changes' ' + ( + cd "$cli" &&
This doesn't look like enough indentation. The tests normally get a hard tab indent at each level.
+ touch file.txt && + p4 add file.txt && + p4 submit -d "Add file.txt" && + for i in 0 1 2 3 4 5 6 7 8 9 + do + touch outer$i.txt && + p4 add outer$i.txt && + p4 submit -d "Adding outer$i.txt" && + for j in 0 1 2 3 4 5 6 7 8 9 + do + p4 edit file.txt && + echo $i$j > file.txt &&
Please put the file argument immediately after the redirection, i.e. echo $i$j >file.txt && (Which you've done below in fact).
+ p4 submit -d "Commit $i$j" + done + done + ) +' + +test_expect_success 'Clone the repo' ' + git p4 clone --dest="$git" --changes-block-size=10 --verbose //depot@all +' + +test_expect_success 'All files are present' ' + echo file.txt >expected && + test_write_lines outer0.txt outer1.txt outer2.txt outer3.txt outer4.txt >>expected && + test_write_lines outer5.txt outer6.txt outer7.txt outer8.txt outer9.txt >>expected && + ls "$git" >current && + test_cmp expected current +' + +test_expect_success 'file.txt is correct' ' + echo 99 >expected && + test_cmp expected "$git/file.txt" +' + +test_expect_success 'Correct number of commits' ' + (cd "$git"; git log --oneline) >log &&
Use "&&" rather than ";"
+ test_line_count = 111 log +' + +test_expect_success 'Previous version of file.txt is correct' ' + (cd "$git"; git checkout HEAD^^) &&
As above.
+ echo 97 >expected && + test_cmp expected "$git/file.txt" +' + +test_expect_success 'kill p4d' ' + kill_p4d +' + +test_done
Looks good other than that (+Junio's comments). Thanks! Luke -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
