Re: [PATCH v3 3/7] t3422: new testcases for checking when incompatible options passed

> Elijah Newren <newren@gmail.com> writes:
>
>> +     git checkout B &&
>> +     # This is indented with HT SP HT.
>> +     echo "          foo();" >>foo &&
>
> I often wonder, whenever I see a need for a comment like this, if
> saying the same thing in code to make it visible is cleaner and less
> error prone way to do so, i.e. e.g.
>
>         echo "_ _foo();" | tr "_" "\011" >>foo &&

>> +# Rebase has lots of useful options like --whitepsace=fix, which are
>> +# actually all built in terms of flags to git-am.  Since neither
>> +# --merge nor --interactive (nor any options that imply those two) use
>> +# git-am, using them together will result in flags like --whitespace=fix
>> +# being ignored.  Make sure rebase warns the user and aborts instead.
>> +#
>> +
>> +test_run_rebase () {
>> +     opt=$1
>> +     shift
>> +     test_expect_failure "$opt incompatible with --merge" "
>> +             git checkout B^0 &&
>> +             test_must_fail git rebase $opt --merge A
>> +     "
>> +
>> +     test_expect_failure "$opt incompatible with --strategy=ours" "
>> +             git checkout B^0 &&
>> +             test_must_fail git rebase $opt --strategy=ours A
>> +     "
>> +
>> +     test_expect_failure "$opt incompatible with --strategy-option=ours" "
>> +             git checkout B^0 &&
>> +             test_must_fail git rebase $opt --strategy=ours A
>> +     "
>> +
>> +     test_expect_failure "$opt incompatible with --interactive" "
>> +             git checkout B^0 &&
>> +             test_must_fail git rebase $opt --interactive A
>> +     "
>> +
>> +     test_expect_failure "$opt incompatible with --exec" "
>> +             git checkout B^0 &&
>> +             test_must_fail git rebase $opt --exec 'true' A
>> +     "
>> +
>> +}
>> +
>> +test_run_rebase --whitespace=fix
>> +test_run_rebase --ignore-whitespace
>> +test_run_rebase --committer-date-is-author-date
>> +test_run_rebase -C4
>
> I happen to be from old school and "rebase" primarily means
> "format-patch piped to am" in my mind, so from that point of view,
> "test_run_rebase --OPT" that says "--OPT which is a valid option for
> the primary operating mode of rebase does not work with the other
> exotic modes of the command" is not all that bad, but I do not think
> that worldview holds for many people in general.  Perhaps calling it
> something like "test_rebase_am_only" makes the intent clearer?