{"thread":{"id":"41904","subject":"[PATCH v2 0/7] t5520: tests for --[no-]autostash option","startedAt":"2016-04-02T17:58:25Z","lastAt":"2016-04-04T17:07:55Z","messageCount":20,"participants":["Mehul Jain","Johannes Sixt","Eric Sunshine","Matthieu Moy","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":7},"messages":[{"id":"282519","messageId":"1459619912-5445-1-git-send-email-mehul.jain2029@gmail.com","threadId":"41904","inReplyTo":null,"subject":"[PATCH v2 0/7] t5520: tests for --[no-]autostash option","fromName":"Mehul Jain","fromEmail":"mehul.jain2029@gmail.com","sentAt":"2016-04-02T17:58:25Z","receivedAt":"2016-04-02T17:58:25Z","isPatch":true,"sender":{"key":"mehul.jain2029@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14936539?v=4"},"body":"The following series is applicable on mj/pull-rebase-autostash.\n\nThanks Eric and Junio for there comments on previous version[1]\n\nChanges made vs v1:\n        * [Patch v1 4/5] is broken into three patches to increase\n\t\t  readability of the patches.\n\n\t\t* [Patch 4/5] Factor out code in two functions \n\t\t  test_pull_autostash() and test_pull_autostash_fail()\n\t\t  instead of test_rebase_autostash() and \n\t\t  test_rebase_no_autostash(). This leads to further \n\t\t  simplification of code.\n\t\t  \n\t\t  Also removed two for-loops as they didn't provided\n\t\t  the simplicity intended for.\n\t\t  \n\t\t  For-loop was over-intended. Corrected it.\n\n\t\t* Commit message for patches 1/5, 2/5, 3/5 are improved\n\t\t  as suggested by Eric in the previous round.\n\nHere's interdiff with v1:\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 4da9e52..bed75f5 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -9,22 +9,22 @@ modify () {\n \tmv \"$2.x\" \"$2\"\n }\n \n-test_rebase_autostash () {\n+test_pull_autostash () {\n \tgit reset --hard before-rebase &&\n \techo dirty >new_file &&\n \tgit add new_file &&\n-\tgit pull --rebase --autostash . copy &&\n+\tgit pull $@ . copy &&\n \ttest_cmp_rev HEAD^ copy &&\n \ttest \"$(cat new_file)\" = dirty &&\n \ttest \"$(cat file)\" = \"modified again\"\n }\n \n-test_rebase_no_autostash () {\n+test_pull_autostash_fail () {\n \tgit reset --hard before-rebase &&\n \techo dirty >new_file &&\n \tgit add new_file &&\n-\ttest_must_fail git pull --rebase --no-autostash . copy 2>err &&\n-\ttest_i18ngrep \"Cannot pull with rebase: Your index contains uncommitted changes.\" err\n+\ttest_must_fail git pull $@ . copy 2>err &&\n+\ttest_i18ngrep \"uncommitted changes.\" err\n }\n \n test_expect_success setup '\n@@ -265,48 +265,46 @@ test_expect_success '--rebase fails with multiple branches' '\n \n test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' '\n \ttest_config rebase.autostash true &&\n-\tgit reset --hard before-rebase &&\n-\techo dirty >new_file &&\n-\tgit add new_file &&\n-\tgit pull --rebase . copy &&\n-\ttest_cmp_rev HEAD^ copy &&\n-\ttest \"$(cat new_file)\" = dirty &&\n-\ttest \"$(cat file)\" = \"modified again\"\n+\ttest_pull_autostash --rebase\n+'\n+\n+test_expect_success 'pull --rebase --autostash & rebase.autostash=true' '\n+\ttest_config rebase.autostash true &&\n+\ttest_pull_autostash --rebase --autostash\n '\n \n-for i in true false\n-\tdo\n-\t\ttest_expect_success \"pull --rebase --autostash & rebase.autostash=$i\" '\n-\t\t\ttest_config rebase.autostash $i &&\n-\t\t\ttest_rebase_autostash\n-\t\t'\n-\tdone\n+test_expect_success 'pull --rebase --autostash & rebase.autostash=false' '\n+\ttest_config rebase.autostash false &&\n+\ttest_pull_autostash --rebase --autostash\n+'\n \n-test_expect_success 'pull --rebase: --autostash & rebase.autostash unset' '\n+test_expect_success 'pull --rebase --autostash & rebase.autostash unset' '\n \ttest_unconfig rebase.autostash &&\n-\ttest_rebase_autostash\n+\ttest_pull_autostash --rebase --autostash\n+'\n+\n+test_expect_success 'pull --rebase --no-autostash & rebase.autostash=true' '\n+\ttest_config rebase.autostash true &&\n+\ttest_pull_autostash_fail --rebase --no-autostash\n '\n \n-for i in true false\n-\tdo\n-\t\ttest_expect_success \"pull --rebase --no-autostash & rebase.autostash=$i\" '\n-\t\t\ttest_config rebase.autostash $i &&\n-\t\t\ttest_rebase_no_autostash\n-\t\t'\n-\tdone\n+test_expect_success 'pull --rebase --no-autostash & rebase.autostash=false' '\n+\ttest_config rebase.autostash false &&\n+\ttest_pull_autostash_fail --rebase --no-autostash\n+'\n \n test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '\n \ttest_unconfig rebase.autostash &&\n-\ttest_rebase_no_autostash\n+\ttest_pull_autostash_fail --rebase --no-autostash\n '\n \n for i in --autostash --no-autostash\n-\tdo\n-\t\ttest_expect_success \"pull $i (without --rebase) is illegal\" '\n-\t\t\ttest_must_fail git pull $i . copy 2>actual &&\n-\t\t\ttest_i18ngrep \"only valid with --rebase\" actual\n-\t\t'\n-\tdone\n+do\n+\ttest_expect_success \"pull $i (without --rebase) is illegal\" '\n+\t\ttest_must_fail git pull $i . copy 2>err &&\n+\t\ttest_i18ngrep \"only valid with --rebase\" err\n+\t'\n+done\n \n test_expect_success 'pull.rebase' '\n \tgit reset --hard before-rebase &&\n@@ -318,22 +316,12 @@ test_expect_success 'pull.rebase' '\n \n test_expect_success 'pull --autostash & pull.rebase=true' '\n \ttest_config pull.rebase true &&\n-\tgit reset --hard before-rebase &&\n-\techo dirty >new_file &&\n-\tgit add new_file &&\n-\tgit pull --autostash . copy &&\n-\ttest_cmp_rev HEAD^ copy &&\n-\ttest \"$(cat new_file)\" = dirty &&\n-\ttest \"$(cat file)\" = \"modified again\"\n+\ttest_pull_autostash --autostash\n '\n \n test_expect_success 'pull --no-autostash & pull.rebase=true' '\n \ttest_config pull.rebase true &&\n-\tgit reset --hard before-rebase &&\n-\techo dirty >new_file &&\n-\tgit add new_file &&\n-\ttest_must_fail git pull --no-autostash . copy 2>err &&\n-\ttest_i18ngrep \"Cannot pull with rebase: Your index contains uncommitted changes.\" err\n+\ttest_pull_autostash_fail --no-autostash\n '\n \n test_expect_success 'branch.to-rebase.rebase' '\n\n\nMehul Jain (7):\n  t5520: use consistent capitalization in test titles\n  t5520: ensure consistent test conditions\n  t5520: use better test to check stderr output\n  t5520: factor out common code\n  t5520: factor out common code\n  t5520: reduce commom lines of code\n  t5520: test --[no-]autostash with pull.rebase=true\n\n t/t5520-pull.sh | 102 +++++++++++++++++++++++++-------------------------------\n 1 file changed, 46 insertions(+), 56 deletions(-)\n\n-- \n2.7.1.340.g69eb491.dirty\n\n[1]:http://thread.gmane.org/gmane.comp.version-control.git/290134\n\nThanks,\nMehul\n"},{"id":"282520","messageId":"1459619912-5445-2-git-send-email-mehul.jain2029@gmail.com","threadId":"41904","inReplyTo":"1459619912-5445-1-git-send-email-mehul.jain2029@gmail.com","subject":"[PATCH v2 1/7] t5520: use consistent capitalization in test titles","fromName":"Mehul Jain","fromEmail":"mehul.jain2029@gmail.com","sentAt":"2016-04-02T17:58:26Z","receivedAt":"2016-04-02T17:58:26Z","isPatch":true,"sender":{"key":"mehul.jain2029@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14936539?v=4"},"body":"Signed-off-by: Mehul Jain <mehul.jain2029@gmail.com>\n---\n t/t5520-pull.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 745e59e..5be39df 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -267,7 +267,7 @@ test_expect_success 'pull --rebase --autostash & rebase.autostash=true' '\n \ttest \"$(cat file)\" = \"modified again\"\n '\n \n-test_expect_success 'pull --rebase --autostash & rebase.autoStash=false' '\n+test_expect_success 'pull --rebase --autostash & rebase.autostash=false' '\n \ttest_config rebase.autostash false &&\n \tgit reset --hard before-rebase &&\n \techo dirty >new_file &&\n@@ -278,7 +278,7 @@ test_expect_success 'pull --rebase --autostash & rebase.autoStash=false' '\n \ttest \"$(cat file)\" = \"modified again\"\n '\n \n-test_expect_success 'pull --rebase: --autostash & rebase.autoStash unset' '\n+test_expect_success 'pull --rebase: --autostash & rebase.autostash unset' '\n \tgit reset --hard before-rebase &&\n \techo dirty >new_file &&\n \tgit add new_file &&\n-- \n2.7.1.340.g69eb491.dirty\n"},{"id":"282521","messageId":"1459619912-5445-3-git-send-email-mehul.jain2029@gmail.com","threadId":"41904","inReplyTo":"1459619912-5445-1-git-send-email-mehul.jain2029@gmail.com","subject":"[PATCH v2 2/7] t5520: ensure consistent test conditions","fromName":"Mehul Jain","fromEmail":"mehul.jain2029@gmail.com","sentAt":"2016-04-02T17:58:27Z","receivedAt":"2016-04-02T17:58:27Z","isPatch":true,"sender":{"key":"mehul.jain2029@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14936539?v=4"},"body":"Test title says that tests are done with rebase.autostash unset,\nbut does not take any action to make sure that it is indeed unset.\nThis may lead to test failure if future changes somehow pollutes\nthe configuration globally.\n\nEnsure consistent test conditions by explicitly unsetting\nrebase.autostash.\n\nSigned-off-by: Mehul Jain <mehul.jain2029@gmail.com>\n---\n t/t5520-pull.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 5be39df..9ee2218 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -279,6 +279,7 @@ test_expect_success 'pull --rebase --autostash & rebase.autostash=false' '\n '\n \n test_expect_success 'pull --rebase: --autostash & rebase.autostash unset' '\n+\ttest_unconfig rebase.autostash &&\n \tgit reset --hard before-rebase &&\n \techo dirty >new_file &&\n \tgit add new_file &&\n@@ -307,6 +308,7 @@ test_expect_success 'pull --rebase --no-autostash & rebase.autostash=false' '\n '\n \n test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '\n+\ttest_unconfig rebase.autostash &&\n \tgit reset --hard before-rebase &&\n \techo dirty >new_file &&\n \tgit add new_file &&\n-- \n2.7.1.340.g69eb491.dirty\n"},{"id":"282522","messageId":"1459619912-5445-4-git-send-email-mehul.jain2029@gmail.com","threadId":"41904","inReplyTo":"1459619912-5445-1-git-send-email-mehul.jain2029@gmail.com","subject":"[PATCH v2 3/7] t5520: use better test to check stderr output","fromName":"Mehul Jain","fromEmail":"mehul.jain2029@gmail.com","sentAt":"2016-04-02T17:58:28Z","receivedAt":"2016-04-02T17:58:28Z","isPatch":true,"sender":{"key":"mehul.jain2029@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14936539?v=4"},"body":"Checking stderr output using test_i18ncmp may lead to test failure as\nsome shells write trace output to stderr when run under 'set -x'.\n\nUse test_i18ngrep instead of test_i18ncmp.\n\nSigned-off-by: Mehul Jain <mehul.jain2029@gmail.com>\n---\n t/t5520-pull.sh | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 9ee2218..d03cb84 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -317,15 +317,13 @@ test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '\n '\n \n test_expect_success 'pull --autostash (without --rebase) should error out' '\n-\ttest_must_fail git pull --autostash . copy 2>actual &&\n-\techo \"fatal: --[no-]autostash option is only valid with --rebase.\" >expect &&\n-\ttest_i18ncmp actual expect\n+\ttest_must_fail git pull --autostash . copy 2>err &&\n+\ttest_i18ngrep \"only valid with --rebase\" err\n '\n \n test_expect_success 'pull --no-autostash (without --rebase) should error out' '\n-\ttest_must_fail git pull --no-autostash . copy 2>actual &&\n-\techo \"fatal: --[no-]autostash option is only valid with --rebase.\" >expect &&\n-\ttest_i18ncmp actual expect\n+\ttest_must_fail git pull --no-autostash . copy 2>err &&\n+\ttest_i18ngrep \"only valid with --rebase\" err\n '\n \n test_expect_success 'pull.rebase' '\n-- \n2.7.1.340.g69eb491.dirty\n"},{"id":"282523","messageId":"1459619912-5445-5-git-send-email-mehul.jain2029@gmail.com","threadId":"41904","inReplyTo":"1459619912-5445-1-git-send-email-mehul.jain2029@gmail.com","subject":"[PATCH v2 4/7] t5520: factor out common code","fromName":"Mehul Jain","fromEmail":"mehul.jain2029@gmail.com","sentAt":"2016-04-02T17:58:29Z","receivedAt":"2016-04-02T17:58:29Z","isPatch":true,"sender":{"key":"mehul.jain2029@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14936539?v=4"},"body":"Four tests contains repetitive lines of code.\n\nFactor out common code into test_pull_autostash() and then call it in\nthese tests.\n\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Mehul Jain <mehul.jain2029@gmail.com>\n---\n t/t5520-pull.sh | 44 +++++++++++++++-----------------------------\n 1 file changed, 15 insertions(+), 29 deletions(-)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex d03cb84..ac063c2 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -9,6 +9,16 @@ modify () {\n \tmv \"$2.x\" \"$2\"\n }\n \n+test_pull_autostash () {\n+\tgit reset --hard before-rebase &&\n+\techo dirty >new_file &&\n+\tgit add new_file &&\n+\tgit pull $@ . copy &&\n+\ttest_cmp_rev HEAD^ copy &&\n+\ttest \"$(cat new_file)\" = dirty &&\n+\ttest \"$(cat file)\" = \"modified again\"\n+}\n+\n test_expect_success setup '\n \techo file >file &&\n \tgit add file &&\n@@ -247,46 +257,22 @@ test_expect_success '--rebase fails with multiple branches' '\n \n test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' '\n \ttest_config rebase.autostash true &&\n-\tgit reset --hard before-rebase &&\n-\techo dirty >new_file &&\n-\tgit add new_file &&\n-\tgit pull --rebase . copy &&\n-\ttest_cmp_rev HEAD^ copy &&\n-\ttest \"$(cat new_file)\" = dirty &&\n-\ttest \"$(cat file)\" = \"modified again\"\n+\ttest_pull_autostash --rebase\n '\n \n test_expect_success 'pull --rebase --autostash & rebase.autostash=true' '\n \ttest_config rebase.autostash true &&\n-\tgit reset --hard before-rebase &&\n-\techo dirty >new_file &&\n-\tgit add new_file &&\n-\tgit pull --rebase --autostash . copy &&\n-\ttest_cmp_rev HEAD^ copy &&\n-\ttest \"$(cat new_file)\" = dirty &&\n-\ttest \"$(cat file)\" = \"modified again\"\n+\ttest_pull_autostash --rebase --autostash\n '\n \n test_expect_success 'pull --rebase --autostash & rebase.autostash=false' '\n \ttest_config rebase.autostash false &&\n-\tgit reset --hard before-rebase &&\n-\techo dirty >new_file &&\n-\tgit add new_file &&\n-\tgit pull --rebase --autostash . copy &&\n-\ttest_cmp_rev HEAD^ copy &&\n-\ttest \"$(cat new_file)\" = dirty &&\n-\ttest \"$(cat file)\" = \"modified again\"\n+\ttest_pull_autostash --rebase --autostash\n '\n \n-test_expect_success 'pull --rebase: --autostash & rebase.autostash unset' '\n+test_expect_success 'pull --rebase --autostash & rebase.autostash unset' '\n \ttest_unconfig rebase.autostash &&\n-\tgit reset --hard before-rebase &&\n-\techo dirty >new_file &&\n-\tgit add new_file &&\n-\tgit pull --rebase --autostash . copy &&\n-\ttest_cmp_rev HEAD^ copy &&\n-\ttest \"$(cat new_file)\" = dirty &&\n-\ttest \"$(cat file)\" = \"modified again\"\n+\ttest_pull_autostash --rebase --autostash\n '\n \n test_expect_success 'pull --rebase --no-autostash & rebase.autostash=true' '\n-- \n2.7.1.340.g69eb491.dirty\n"},{"id":"282524","messageId":"1459619912-5445-6-git-send-email-mehul.jain2029@gmail.com","threadId":"41904","inReplyTo":"1459619912-5445-1-git-send-email-mehul.jain2029@gmail.com","subject":"[PATCH v2 5/7] t5520: factor out common code","fromName":"Mehul Jain","fromEmail":"mehul.jain2029@gmail.com","sentAt":"2016-04-02T17:58:30Z","receivedAt":"2016-04-02T17:58:30Z","isPatch":true,"sender":{"key":"mehul.jain2029@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14936539?v=4"},"body":"Three tests contains repetitive lines of code.\n\nFactor out common code into test_pull_autostash_fail() and then call it in\nthese tests.\n\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Mehul Jain <mehul.jain2029@gmail.com>\n---\n t/t5520-pull.sh | 26 +++++++++++---------------\n 1 file changed, 11 insertions(+), 15 deletions(-)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex ac063c2..fb9f845 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -19,6 +19,14 @@ test_pull_autostash () {\n \ttest \"$(cat file)\" = \"modified again\"\n }\n \n+test_pull_autostash_fail () {\n+\tgit reset --hard before-rebase &&\n+\techo dirty >new_file &&\n+\tgit add new_file &&\n+\ttest_must_fail git pull $@ . copy 2>err &&\n+\ttest_i18ngrep \"uncommitted changes.\" err\n+}\n+\n test_expect_success setup '\n \techo file >file &&\n \tgit add file &&\n@@ -277,29 +285,17 @@ test_expect_success 'pull --rebase --autostash & rebase.autostash unset' '\n \n test_expect_success 'pull --rebase --no-autostash & rebase.autostash=true' '\n \ttest_config rebase.autostash true &&\n-\tgit reset --hard before-rebase &&\n-\techo dirty >new_file &&\n-\tgit add new_file &&\n-\ttest_must_fail git pull --rebase --no-autostash . copy 2>err &&\n-\ttest_i18ngrep \"Cannot pull with rebase: Your index contains uncommitted changes.\" err\n+\ttest_pull_autostash_fail --rebase --no-autostash\n '\n \n test_expect_success 'pull --rebase --no-autostash & rebase.autostash=false' '\n \ttest_config rebase.autostash false &&\n-\tgit reset --hard before-rebase &&\n-\techo dirty >new_file &&\n-\tgit add new_file &&\n-\ttest_must_fail git pull --rebase --no-autostash . copy 2>err &&\n-\ttest_i18ngrep \"Cannot pull with rebase: Your index contains uncommitted changes.\" err\n+\ttest_pull_autostash_fail --rebase --no-autostash\n '\n \n test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '\n \ttest_unconfig rebase.autostash &&\n-\tgit reset --hard before-rebase &&\n-\techo dirty >new_file &&\n-\tgit add new_file &&\n-\ttest_must_fail git pull --rebase --no-autostash . copy 2>err &&\n-\ttest_i18ngrep \"Cannot pull with rebase: Your index contains uncommitted changes.\" err\n+\ttest_pull_autostash_fail --rebase --no-autostash\n '\n \n test_expect_success 'pull --autostash (without --rebase) should error out' '\n-- \n2.7.1.340.g69eb491.dirty\n"},{"id":"282525","messageId":"1459619912-5445-7-git-send-email-mehul.jain2029@gmail.com","threadId":"41904","inReplyTo":"1459619912-5445-1-git-send-email-mehul.jain2029@gmail.com","subject":"[PATCH v2 6/7] t5520: reduce commom lines of code","fromName":"Mehul Jain","fromEmail":"mehul.jain2029@gmail.com","sentAt":"2016-04-02T17:58:31Z","receivedAt":"2016-04-02T17:58:31Z","isPatch":true,"sender":{"key":"mehul.jain2029@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14936539?v=4"},"body":"These two tests are almost similar and thus can be folded in a for-loop.\n\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Mehul Jain <mehul.jain2029@gmail.com>\n---\n t/t5520-pull.sh | 16 +++++++---------\n 1 file changed, 7 insertions(+), 9 deletions(-)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex fb9f845..e12af96 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -298,15 +298,13 @@ test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '\n \ttest_pull_autostash_fail --rebase --no-autostash\n '\n \n-test_expect_success 'pull --autostash (without --rebase) should error out' '\n-\ttest_must_fail git pull --autostash . copy 2>err &&\n-\ttest_i18ngrep \"only valid with --rebase\" err\n-'\n-\n-test_expect_success 'pull --no-autostash (without --rebase) should error out' '\n-\ttest_must_fail git pull --no-autostash . copy 2>err &&\n-\ttest_i18ngrep \"only valid with --rebase\" err\n-'\n+for i in --autostash --no-autostash\n+do\n+\ttest_expect_success \"pull $i (without --rebase) is illegal\" '\n+\t\ttest_must_fail git pull $i . copy 2>err &&\n+\t\ttest_i18ngrep \"only valid with --rebase\" err\n+\t'\n+done\n \n test_expect_success 'pull.rebase' '\n \tgit reset --hard before-rebase &&\n-- \n2.7.1.340.g69eb491.dirty\n"},{"id":"282526","messageId":"1459619912-5445-8-git-send-email-mehul.jain2029@gmail.com","threadId":"41904","inReplyTo":"1459619912-5445-1-git-send-email-mehul.jain2029@gmail.com","subject":"[PATCH v2 7/7] t5520: test --[no-]autostash with pull.rebase=true","fromName":"Mehul Jain","fromEmail":"mehul.jain2029@gmail.com","sentAt":"2016-04-02T17:58:32Z","receivedAt":"2016-04-02T17:58:32Z","isPatch":true,"sender":{"key":"mehul.jain2029@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14936539?v=4"},"body":"\"--[no-]autostash\" option for git-pull is only valid in rebase mode(\ni.e. either --rebase should be used or pull.rebase=true). Existing\ntests already check the cases when --rebase is used but fails to check\nfor pull.rebase=true case.\n\nAdd two new tests to check that --[no-]autostash option works with\npull.rebase=true.\n\nSigned-off-by: Mehul Jain <mehul.jain2029@gmail.com>\n---\n t/t5520-pull.sh | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex e12af96..bed75f5 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -314,6 +314,16 @@ test_expect_success 'pull.rebase' '\n \ttest new = \"$(git show HEAD:file2)\"\n '\n \n+test_expect_success 'pull --autostash & pull.rebase=true' '\n+\ttest_config pull.rebase true &&\n+\ttest_pull_autostash --autostash\n+'\n+\n+test_expect_success 'pull --no-autostash & pull.rebase=true' '\n+\ttest_config pull.rebase true &&\n+\ttest_pull_autostash_fail --no-autostash\n+'\n+\n test_expect_success 'branch.to-rebase.rebase' '\n \tgit reset --hard before-rebase &&\n \ttest_config branch.to-rebase.rebase true &&\n-- \n2.7.1.340.g69eb491.dirty\n"},{"id":"282527","messageId":"5700145E.4060308@kdbg.org","threadId":"41904","inReplyTo":"1459619912-5445-7-git-send-email-mehul.jain2029@gmail.com","subject":"Re: [PATCH v2 6/7] t5520: reduce commom lines of code","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-04-02T18:50:06Z","receivedAt":"2016-04-02T18:50:06Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 02.04.2016 um 19:58 schrieb Mehul Jain:\n> These two tests are almost similar and thus can be folded in a for-loop.\n>\n> Helped-by: Eric Sunshine <sunshine@sunshineco.com>\n> Signed-off-by: Mehul Jain <mehul.jain2029@gmail.com>\n> ---\n>   t/t5520-pull.sh | 16 +++++++---------\n>   1 file changed, 7 insertions(+), 9 deletions(-)\n>\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index fb9f845..e12af96 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -298,15 +298,13 @@ test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '\n>   \ttest_pull_autostash_fail --rebase --no-autostash\n>   '\n>\n> -test_expect_success 'pull --autostash (without --rebase) should error out' '\n> -\ttest_must_fail git pull --autostash . copy 2>err &&\n> -\ttest_i18ngrep \"only valid with --rebase\" err\n> -'\n> -\n> -test_expect_success 'pull --no-autostash (without --rebase) should error out' '\n> -\ttest_must_fail git pull --no-autostash . copy 2>err &&\n> -\ttest_i18ngrep \"only valid with --rebase\" err\n> -'\n> +for i in --autostash --no-autostash\n> +do\n> +\ttest_expect_success \"pull $i (without --rebase) is illegal\" '\n> +\t\ttest_must_fail git pull $i . copy 2>err &&\n> +\t\ttest_i18ngrep \"only valid with --rebase\" err\n> +\t'\n> +done\n\nHm. If the implementation of test_expect_success uses the variable, too, \nits value is lost when the test snippet runs. Fortunately, it does not.\n\nYou can make this code a bit more robust by using double-quotes around \nthe test code so that $i is expanded before test_expect_success is \nevaluated.\n\nYou could also change the variable name, but to be sufficiently safe, \nyou would have to use an unsightly long name. 'opt' would be just as bad \nas 'i'.\n\n>\n>   test_expect_success 'pull.rebase' '\n>   \tgit reset --hard before-rebase &&\n>\n\n-- Hannes\n"},{"id":"282553","messageId":"CA+DCAeT4rF2dqL9iU9WYQJuxiSYstY9AaT2Hc7OvhmFNyKEgAg@mail.gmail.com","threadId":"41904","inReplyTo":"5700145E.4060308@kdbg.org","subject":"Re: [PATCH v2 6/7] t5520: reduce commom lines of code","fromName":"Mehul Jain","fromEmail":"mehul.jain2029@gmail.com","sentAt":"2016-04-03T06:24:36Z","receivedAt":"2016-04-03T06:24:36Z","isPatch":true,"sender":{"key":"mehul.jain2029@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14936539?v=4"},"body":"On Sun, Apr 3, 2016 at 12:20 AM, Johannes Sixt <j6t@kdbg.org> wrote:\n> Am 02.04.2016 um 19:58 schrieb Mehul Jain:\n>> +for i in --autostash --no-autostash\n>> +do\n>> +       test_expect_success \"pull $i (without --rebase) is illegal\" '\n>> +               test_must_fail git pull $i . copy 2>err &&\n>> +               test_i18ngrep \"only valid with --rebase\" err\n>> +       '\n>> +done\n>\n>\n> Hm. If the implementation of test_expect_success uses the variable, too, its\n> value is lost when the test snippet runs. Fortunately, it does not.\n>\n> You can make this code a bit more robust by using double-quotes around the\n> test code so that $i is expanded before test_expect_success is evaluated.\n\nI think that the current format is preferred over the one you suggest.\nHere[1] Junio\nhas given a descriptive explanation.\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/283350/focus=284769\n\nThanks,\nMehul\n"},{"id":"282555","messageId":"5700BC52.1040404@kdbg.org","threadId":"41904","inReplyTo":"CA+DCAeT4rF2dqL9iU9WYQJuxiSYstY9AaT2Hc7OvhmFNyKEgAg@mail.gmail.com","subject":"Re: [PATCH v2 6/7] t5520: reduce commom lines of code","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-04-03T06:46:42Z","receivedAt":"2016-04-03T06:46:42Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 03.04.2016 um 08:24 schrieb Mehul Jain:\n> On Sun, Apr 3, 2016 at 12:20 AM, Johannes Sixt <j6t@kdbg.org> wrote:\n>> Am 02.04.2016 um 19:58 schrieb Mehul Jain:\n>>> +for i in --autostash --no-autostash\n>>> +do\n>>> +       test_expect_success \"pull $i (without --rebase) is illegal\" '\n>>> +               test_must_fail git pull $i . copy 2>err &&\n>>> +               test_i18ngrep \"only valid with --rebase\" err\n>>> +       '\n>>> +done\n>>\n>>\n>> Hm. If the implementation of test_expect_success uses the variable, too, its\n>> value is lost when the test snippet runs. Fortunately, it does not.\n>>\n>> You can make this code a bit more robust by using double-quotes around the\n>> test code so that $i is expanded before test_expect_success is evaluated.\n>\n> I think that the current format is preferred over the one you suggest.\n> Here[1] Junio\n> has given a descriptive explanation.\n>\n> [1]: http://thread.gmane.org/gmane.comp.version-control.git/283350/focus=284769\n\nJunio has a point there, of course.\n\nIn this light, I suggest that you use a more verbose variable name.\n\n-- Hannes\n"},{"id":"282559","messageId":"1459673257-6344-1-git-send-email-mehul.jain2029@gmail.com","threadId":"41904","inReplyTo":"1459619912-5445-1-git-send-email-mehul.jain2029@gmail.com","subject":"[PATCH v2 6/7] t5520: reduce commom lines of code","fromName":"Mehul Jain","fromEmail":"mehul.jain2029@gmail.com","sentAt":"2016-04-03T08:47:37Z","receivedAt":"2016-04-03T08:47:37Z","isPatch":true,"sender":{"key":"mehul.jain2029@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14936539?v=4"},"body":"These two tests are almost similar and thus can be folded in a for-loop.\n\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Mehul Jain <mehul.jain2029@gmail.com>\n---\n t/t5520-pull.sh | 16 +++++++---------\n 1 file changed, 7 insertions(+), 9 deletions(-)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex fb9f845..e12af96 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -298,15 +298,13 @@ test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '\n \ttest_pull_autostash_fail --rebase --no-autostash\n '\n \n-test_expect_success 'pull --autostash (without --rebase) should error out' '\n-\ttest_must_fail git pull --autostash . copy 2>err &&\n-\ttest_i18ngrep \"only valid with --rebase\" err\n-'\n-\n-test_expect_success 'pull --no-autostash (without --rebase) should error out' '\n-\ttest_must_fail git pull --no-autostash . copy 2>err &&\n-\ttest_i18ngrep \"only valid with --rebase\" err\n-'\n+for autostash_flag in --autostash --no-autostash\n+do\n+\ttest_expect_success \"pull $autostash_flag (without --rebase) is illegal\" '\n+\t\ttest_must_fail git pull $autostash_flag . copy 2>err &&\n+\t\ttest_i18ngrep \"only valid with --rebase\" err\n+\t'\n+done\n \n test_expect_success 'pull.rebase' '\n \tgit reset --hard before-rebase &&\n-- \n2.7.1.340.g69eb491.dirty\n"},{"id":"282573","messageId":"CAPig+cSofS_ozEY2N1k4fjgubq-J=3k870UMvx0-j55LPaMoRg@mail.gmail.com","threadId":"41904","inReplyTo":"1459619912-5445-5-git-send-email-mehul.jain2029@gmail.com","subject":"Re: [PATCH v2 4/7] t5520: factor out common code","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-03T20:03:20Z","receivedAt":"2016-04-03T20:03:20Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Apr 2, 2016 at 1:58 PM, Mehul Jain <mehul.jain2029@gmail.com> wrote:\n> t5520: factor out common code\n\nTo distinguish this title from that of patch 5/7, you could say:\n\n    t5520: factor out common \"successful autostash\" code\n\n> Four tests contains repetitive lines of code.\n>\n> Factor out common code into test_pull_autostash() and then call it in\n> these tests.\n>\n> Helped-by: Eric Sunshine <sunshine@sunshineco.com>\n> Signed-off-by: Mehul Jain <mehul.jain2029@gmail.com>\n> ---\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> @@ -9,6 +9,16 @@ modify () {\n> +test_pull_autostash () {\n> +       git reset --hard before-rebase &&\n> +       echo dirty >new_file &&\n> +       git add new_file &&\n> +       git pull $@ . copy &&\n\nNit: This could just as well be $* rather than $@.\n\n> +       test_cmp_rev HEAD^ copy &&\n> +       test \"$(cat new_file)\" = dirty &&\n> +       test \"$(cat file)\" = \"modified again\"\n> +}\n> @@ -247,46 +257,22 @@ test_expect_success '--rebase fails with multiple branches' '\n>\n>  test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' '\n>         test_config rebase.autostash true &&\n> -       git reset --hard before-rebase &&\n> -       echo dirty >new_file &&\n> -       git add new_file &&\n> -       git pull --rebase . copy &&\n> -       test_cmp_rev HEAD^ copy &&\n> -       test \"$(cat new_file)\" = dirty &&\n> -       test \"$(cat file)\" = \"modified again\"\n> +       test_pull_autostash --rebase\n>  '\n>\n>  test_expect_success 'pull --rebase --autostash & rebase.autostash=true' '\n>         test_config rebase.autostash true &&\n> -       git reset --hard before-rebase &&\n> -       echo dirty >new_file &&\n> -       git add new_file &&\n> -       git pull --rebase --autostash . copy &&\n> -       test_cmp_rev HEAD^ copy &&\n> -       test \"$(cat new_file)\" = dirty &&\n> -       test \"$(cat file)\" = \"modified again\"\n> +       test_pull_autostash --rebase --autostash\n>  '\n>\n>  test_expect_success 'pull --rebase --autostash & rebase.autostash=false' '\n>         test_config rebase.autostash false &&\n> -       git reset --hard before-rebase &&\n> -       echo dirty >new_file &&\n> -       git add new_file &&\n> -       git pull --rebase --autostash . copy &&\n> -       test_cmp_rev HEAD^ copy &&\n> -       test \"$(cat new_file)\" = dirty &&\n> -       test \"$(cat file)\" = \"modified again\"\n> +       test_pull_autostash --rebase --autostash\n>  '\n>\n> -test_expect_success 'pull --rebase: --autostash & rebase.autostash unset' '\n> +test_expect_success 'pull --rebase --autostash & rebase.autostash unset' '\n>         test_unconfig rebase.autostash &&\n> -       git reset --hard before-rebase &&\n> -       echo dirty >new_file &&\n> -       git add new_file &&\n> -       git pull --rebase --autostash . copy &&\n> -       test_cmp_rev HEAD^ copy &&\n> -       test \"$(cat new_file)\" = dirty &&\n> -       test \"$(cat file)\" = \"modified again\"\n> +       test_pull_autostash --rebase --autostash\n>  '\n>\n>  test_expect_success 'pull --rebase --no-autostash & rebase.autostash=true' '\n> --\n> 2.7.1.340.g69eb491.dirty\n"},{"id":"282572","messageId":"CAPig+cS3yhQdTQrX4kC89GjLW==ynEjxatXVFpJo4jkJENOf6w@mail.gmail.com","threadId":"41904","inReplyTo":"1459619912-5445-6-git-send-email-mehul.jain2029@gmail.com","subject":"Re: [PATCH v2 5/7] t5520: factor out common code","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-03T20:05:19Z","receivedAt":"2016-04-03T20:05:19Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Apr 2, 2016 at 1:58 PM, Mehul Jain <mehul.jain2029@gmail.com> wrote:\n> t5520: factor out common code\n\nTo distinguish this title from that of patch 4/7, you could say:\n\n    t5520: factor out common \"failing autostash\" code\n\n> Three tests contains repetitive lines of code.\n>\n> Factor out common code into test_pull_autostash_fail() and then call it in\n> these tests.\n>\n> Helped-by: Eric Sunshine <sunshine@sunshineco.com>\n> Signed-off-by: Mehul Jain <mehul.jain2029@gmail.com>\n> ---\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> @@ -19,6 +19,14 @@ test_pull_autostash () {\n> +test_pull_autostash_fail () {\n> +       git reset --hard before-rebase &&\n> +       echo dirty >new_file &&\n> +       git add new_file &&\n> +       test_must_fail git pull $@ . copy 2>err &&\n\nNit: Same comment as in 4/7: This could just as well be $* rather than $@.\n\n> +       test_i18ngrep \"uncommitted changes.\" err\n> +}\n> @@ -277,29 +285,17 @@ test_expect_success 'pull --rebase --autostash & rebase.autostash unset' '\n>\n>  test_expect_success 'pull --rebase --no-autostash & rebase.autostash=true' '\n>         test_config rebase.autostash true &&\n> -       git reset --hard before-rebase &&\n> -       echo dirty >new_file &&\n> -       git add new_file &&\n> -       test_must_fail git pull --rebase --no-autostash . copy 2>err &&\n> -       test_i18ngrep \"Cannot pull with rebase: Your index contains uncommitted changes.\" err\n> +       test_pull_autostash_fail --rebase --no-autostash\n>  '\n>\n>  test_expect_success 'pull --rebase --no-autostash & rebase.autostash=false' '\n>         test_config rebase.autostash false &&\n> -       git reset --hard before-rebase &&\n> -       echo dirty >new_file &&\n> -       git add new_file &&\n> -       test_must_fail git pull --rebase --no-autostash . copy 2>err &&\n> -       test_i18ngrep \"Cannot pull with rebase: Your index contains uncommitted changes.\" err\n> +       test_pull_autostash_fail --rebase --no-autostash\n>  '\n>\n>  test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '\n>         test_unconfig rebase.autostash &&\n> -       git reset --hard before-rebase &&\n> -       echo dirty >new_file &&\n> -       git add new_file &&\n> -       test_must_fail git pull --rebase --no-autostash . copy 2>err &&\n> -       test_i18ngrep \"Cannot pull with rebase: Your index contains uncommitted changes.\" err\n> +       test_pull_autostash_fail --rebase --no-autostash\n>  '\n>\n>  test_expect_success 'pull --autostash (without --rebase) should error out' '\n> --\n> 2.7.1.340.g69eb491.dirty\n"},{"id":"282574","messageId":"CAPig+cQd93yUhog5h6qBrJEE_g+7-XLpSLQmDqYf7yh7Vr3Z3Q@mail.gmail.com","threadId":"41904","inReplyTo":"1459619912-5445-8-git-send-email-mehul.jain2029@gmail.com","subject":"Re: [PATCH v2 7/7] t5520: test --[no-]autostash with pull.rebase=true","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-03T20:11:07Z","receivedAt":"2016-04-03T20:11:07Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Apr 2, 2016 at 1:58 PM, Mehul Jain <mehul.jain2029@gmail.com> wrote:\n> \"--[no-]autostash\" option for git-pull is only valid in rebase mode(\n\ns/\"--[no-]autostash\"/The --[no-]autostash/\n\nAlso, move the '(' from the end of the line to the beginning of the next line.\n\n> i.e. either --rebase should be used or pull.rebase=true). Existing\n> tests already check the cases when --rebase is used but fails to check\n> for pull.rebase=true case.\n>\n> Add two new tests to check that --[no-]autostash option works with\n> pull.rebase=true.\n\nNicely explained.\n\n> Signed-off-by: Mehul Jain <mehul.jain2029@gmail.com>\n> ---\n>  t/t5520-pull.sh | 10 ++++++++++\n>  1 file changed, 10 insertions(+)\n>\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index e12af96..bed75f5 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -314,6 +314,16 @@ test_expect_success 'pull.rebase' '\n>         test new = \"$(git show HEAD:file2)\"\n>  '\n>\n> +test_expect_success 'pull --autostash & pull.rebase=true' '\n> +       test_config pull.rebase true &&\n> +       test_pull_autostash --autostash\n> +'\n> +\n> +test_expect_success 'pull --no-autostash & pull.rebase=true' '\n> +       test_config pull.rebase true &&\n> +       test_pull_autostash_fail --no-autostash\n> +'\n> +\n>  test_expect_success 'branch.to-rebase.rebase' '\n>         git reset --hard before-rebase &&\n>         test_config branch.to-rebase.rebase true &&\n> --\n> 2.7.1.340.g69eb491.dirty\n"},{"id":"282576","messageId":"CAPig+cR3diDfn893-ExKNZps=C7Z=M7DFAy-zbJzH3wKCmxVeQ@mail.gmail.com","threadId":"41904","inReplyTo":"1459619912-5445-1-git-send-email-mehul.jain2029@gmail.com","subject":"Re: [PATCH v2 0/7] t5520: tests for --[no-]autostash option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-03T20:17:30Z","receivedAt":"2016-04-03T20:17:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Apr 2, 2016 at 1:58 PM, Mehul Jain <mehul.jain2029@gmail.com> wrote:\n> The following series is applicable on mj/pull-rebase-autostash.\n>\n> Changes made vs v1:\n>         * [Patch v1 4/5] is broken into three patches to increase\n>                   readability of the patches.\n>\n>                 * [Patch 4/5] Factor out code in two functions\n>                   test_pull_autostash() and test_pull_autostash_fail()\n>                   instead of test_rebase_autostash() and\n>                   test_rebase_no_autostash(). This leads to further\n>                   simplification of code.\n>\n>                   Also removed two for-loops as they didn't provided\n>                   the simplicity intended for.\n>\n>                   For-loop was over-intended. Corrected it.\n>\n>                 * Commit message for patches 1/5, 2/5, 3/5 are improved\n>                   as suggested by Eric in the previous round.\n\nThanks, this version was a pleasant read, much simpler and easier to\ndigest than the previous round[1]. With or without addressing the few\nminor nits in my review (none of which warrant a re-roll), this entire\nseries is:\n\n    Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>\n\nMehul, feel free to add my Reviewed-by: if you happen to re-roll (or\nJunio can add it if he wants when he picks up the series).\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/290134\n"},{"id":"282603","messageId":"vpqshz125jr.fsf@anie.imag.fr","threadId":"41904","inReplyTo":"1459619912-5445-1-git-send-email-mehul.jain2029@gmail.com","subject":"Re: [PATCH v2 0/7] t5520: tests for --[no-]autostash option","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-04-04T07:31:52Z","receivedAt":"2016-04-04T07:31:52Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Mehul Jain <mehul.jain2029@gmail.com> writes:\n\n> -test_rebase_autostash () {\n> +test_pull_autostash () {\n>  \tgit reset --hard before-rebase &&\n>  \techo dirty >new_file &&\n>  \tgit add new_file &&\n> -\tgit pull --rebase --autostash . copy &&\n> +\tgit pull $@ . copy &&\n\nNot strictly needed here, but I'd write \"$@\" (with the double-quotes)\nwhich is the robust way to say \"transmit all my arguments without\nwhitespace interpretation\".\n\nI don't mind for this patch since there's no whitespace to interpret,\nbut some people (sysadmins ;-) ) have the bad habit of writting $@, $*\nor \"$*\" in wrapper scripts and it breaks when you call them with spaces\nso it's better to take good habits IHMO.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"282622","messageId":"CA+DCAeQaS0P=Rntv5xY97MQ-j_1ji6O+MgvmcnjVmxC3KsNfRw@mail.gmail.com","threadId":"41904","inReplyTo":"vpqshz125jr.fsf@anie.imag.fr","subject":"Re: [PATCH v2 0/7] t5520: tests for --[no-]autostash option","fromName":"Mehul Jain","fromEmail":"mehul.jain2029@gmail.com","sentAt":"2016-04-04T16:58:02Z","receivedAt":"2016-04-04T16:58:02Z","isPatch":true,"sender":{"key":"mehul.jain2029@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14936539?v=4"},"body":"On Mon, Apr 4, 2016 at 1:01 PM, Matthieu Moy\n<Matthieu.Moy@grenoble-inp.fr> wrote:\n> Mehul Jain <mehul.jain2029@gmail.com> writes:\n>\n>> -test_rebase_autostash () {\n>> +test_pull_autostash () {\n>>       git reset --hard before-rebase &&\n>>       echo dirty >new_file &&\n>>       git add new_file &&\n>> -     git pull --rebase --autostash . copy &&\n>> +     git pull $@ . copy &&\n>\n> Not strictly needed here, but I'd write \"$@\" (with the double-quotes)\n> which is the robust way to say \"transmit all my arguments without\n> whitespace interpretation\".\n>\n> I don't mind for this patch since there's no whitespace to interpret,\n> but some people (sysadmins ;-) ) have the bad habit of writting $@, $*\n> or \"$*\" in wrapper scripts and it breaks when you call them with spaces\n> so it's better to take good habits IHMO.\n\nThanks for the suggestion, I will remember it. I'm relatively new to\nshell and therefore didn't know much about the difference\nbetween \"$@\" and $@, $*, \"$*\".\n\nNow that I have read[1][2] about it, it won't be repeated.\n\n[1]: http://unix.stackexchange.com/questions/41571/what-is-the-difference-between-and/94200#94200\n[2]: http://unix.stackexchange.com/questions/131766/why-does-my-shell-script-choke-on-whitespace-or-other-special-characters\n\nThanks,\nMehul\n"},{"id":"282623","messageId":"xmqqmvp95mxl.fsf@gitster.mtv.corp.google.com","threadId":"41904","inReplyTo":"vpqshz125jr.fsf@anie.imag.fr","subject":"Re: [PATCH v2 0/7] t5520: tests for --[no-]autostash option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-04T17:00:22Z","receivedAt":"2016-04-04T17:00:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Mehul Jain <mehul.jain2029@gmail.com> writes:\n>\n>> -test_rebase_autostash () {\n>> +test_pull_autostash () {\n>>  \tgit reset --hard before-rebase &&\n>>  \techo dirty >new_file &&\n>>  \tgit add new_file &&\n>> -\tgit pull --rebase --autostash . copy &&\n>> +\tgit pull $@ . copy &&\n>\n> Not strictly needed here, but I'd write \"$@\" (with the double-quotes)\n> which is the robust way to say \"transmit all my arguments without\n> whitespace interpretation\".\n\nYes, these should be \"$@\" (with the double-quotes).\n\n> I don't mind for this patch since there's no whitespace to interpret,\n> but some people (sysadmins ;-) ) have the bad habit of writting $@, $*\n> or \"$*\" in wrapper scripts and it breaks when you call them with spaces\n> so it's better to take good habits IHMO.\n"},{"id":"282624","messageId":"CA+DCAeQU2eOA7gQ4Q=GTGWeNuAzor0aL4DUFnD+8vYErPc21DA@mail.gmail.com","threadId":"41904","inReplyTo":"xmqqmvp95mxl.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 0/7] t5520: tests for --[no-]autostash option","fromName":"Mehul Jain","fromEmail":"mehul.jain2029@gmail.com","sentAt":"2016-04-04T17:07:55Z","receivedAt":"2016-04-04T17:07:55Z","isPatch":true,"sender":{"key":"mehul.jain2029@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14936539?v=4"},"body":"On Mon, Apr 4, 2016 at 10:30 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n>\n>> Mehul Jain <mehul.jain2029@gmail.com> writes:\n>>\n>>> -test_rebase_autostash () {\n>>> +test_pull_autostash () {\n>>>      git reset --hard before-rebase &&\n>>>      echo dirty >new_file &&\n>>>      git add new_file &&\n>>> -    git pull --rebase --autostash . copy &&\n>>> +    git pull $@ . copy &&\n>>\n>> Not strictly needed here, but I'd write \"$@\" (with the double-quotes)\n>> which is the robust way to say \"transmit all my arguments without\n>> whitespace interpretation\".\n>\n> Yes, these should be \"$@\" (with the double-quotes).\n\nI will do a re-roll then.\n\nThanks,\nMehul\n"}]}