Re: [PATCH 1/2] tests: eliminate unnecessary setup test assertions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 06, 2011 at 03:58:52PM -0500, Jonathan Nieder wrote:

> Two scripts use a different style with this kind of trivial code
> enclosed by a test assertion; fix them.  The usual style is easier to
> read since there is less indentation to keep track of and no need to
> worry about nested quotes; and on the other hand, because the commands
> in question are trivial, it should not make the test suite any worse
> at catching future bugs in git.

Thanks. Glancing at the first few hunks, this change didn't seem like a
big cleanup, but then I got to the hunk with all the ugly '\'' bits. :)

The patch looks correct to me (reviewing with "git diff -b" was a big
help).

> While at it, make some other small tweaks:
> 
>  - spell function definitions with a space before () for consistency
>    with other scripts;

Hmm.

  $ cd t
  $ git grep ' ()' *.sh  | wc -l
  271
  $ git grep '[^ ]()' *.sh  | wc -l
  247

I'm not sure you are making things any more consistent. But I don't
really care either way.

Just for giggles, I was curious who introduced the styles:

  pattern_authors() {
    git grep -n "$@" |
    while IFS=: read file line match; do
      git blame -L "$line,$line" "$file"
    done |
    perl -lpe '/\((.*?) \d+-\d+-\d+/; $_=$1'
  }

  $ pattern_authors ' ()' t/*.sh | sort | uniq -c | sort -rn
  84 Junio C Hamano
  32 Jonathan Nieder
  16 Thomas Rast
  16 Johan Herland
  12 Johannes Sixt
  12 Johannes Schindelin
  ...

  $ pattern_authors '[^ ]()' t/*.sh | sort | uniq -c | sort -rn
  52 Jeff King
  26 Jonathan Nieder
  18 Nguyán ThÃi Ngác Duy
  16 Wincent Colaiuta
  14 Jon Seymour
  10 Tay Ray Chuan
  ...

So there are definitely particular people who prefer different styles
(and I recalled that Junio and I differed on this style point, which is
confirmed here). Interestingly, you are the only person to fall right in
the middle.  I guess that means you are good at emulating surrounding
code. :)

-Peff
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]