Tom Grennan <tmgrennan@xxxxxxxxx> writes: >On Tue, Feb 21, 2012 at 10:33:29PM -0800, Junio C Hamano wrote: >>I know you are imitating the style of surrounding tests that is an older >>parts of this script, but it is an eyesore. More modern tests are written >>like this: >> >> test_expect_success 'label for the test' ' >> cat >expect <<-EOF && >> v0.2.1 >> EOF >> git tag -l ... >actual && >> test_cmp expect actual >> ' >> >>to avoid unnecessary backslash on the first line, and have the preparation >>of test vectore _inside_ test_expect_success. We would eventually want to >>update the older part to the newer style for consistency. >> >>Two possible ways to go about this are (1) have a "pure style" patch at >>the beginning to update older tests to a new style and then add new code >>and new test as a follow-up patch written in modern, or (2) add new code >>and new test in modern, and make a mental note to update the older ones >>after the dust settles. Adding new tests written in older style to a file >>that already has mixed styles is the worst thing you can do. >> >>This comment applies to all the patches in this series with tests. > >I'd prefer, (1) precede each "--exclude" patch with a "pure style" patch >to update the respective tests. However, since this will result in a >lot of conflict with concurrent development; I'll separate the test >patches from the code and documentation. I'll then cycle on rebasing >the style and new test patches until the development of each is >quiescent. Per request, the following series modernize the style of the respective test scripts. The common themes are: - Guard setup with test_expect_success - Single-quoted, tab prefaced test blocks of < 80 cols - Redirect unwanted output - Use a "here" filter for some expect generation I also used pipelines to validate expected results rather than temporary files, i.e. TEST | test_cmp expect - vs. TEST >actual && test_cmp expect actual Since the later three patches have a lot of whitespace change, I've included an alternate, PATCH-w series that filters these for more substantive review. However, even the filtered series is very large causing me to second guess whether such style modernization should be pursued; so, I look forward to your input. Thanks, Tom Grennan (5): t6300 (for-each-ref): modernize style t5512 (ls-remote): modernize style t3200 (branch): modernize style t0040 (parse-options): modernize style t7004 (tag): modernize style t/t7004-tag.sh | 1680 ++++++++++++++++++++++++++------------------------------ 1 files changed, 783 insertions(+), 897 deletions(-) -- 1.7.8 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html