All of lore.kernel.org
 help / color / mirror / Atom feed
From: "brian m. carlson" <sandals@crustytoothpaste.net>
To: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH] tests: consolidate the `file_size` function into `test-lib-functions.sh`
Date: Sat, 7 Nov 2020 01:23:45 +0000	[thread overview]
Message-ID: <20201107012345.GD6252@camp.crustytoothpaste.net> (raw)
In-Reply-To: <pull.780.git.1604711577662.gitgitgadget@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1350 bytes --]

On 2020-11-07 at 01:12:57, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> In 8de7eeb54b6 (compression: unify pack.compression configuration
> parsing, 2016-11-15), we introduced identical copies of the `file_size`
> helper into three test scripts, with the plan to eventually consolidate
> them into a single copy.
> 
> Let's do that, and adjust the function name to adhere to the `test_*`
> naming convention.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

This seems reasonable.  For a moment, I was going to ask why we didn't
just use test -s, but then I remembered that test -s doesn't have that
behavior, Perl's -s does, and you replaced that with the helper for
efficiency on Windows.

I looked at the patch and it seemed pretty straightforward.

>     tests: consolidate the file_size function into test-lib-functions.sh
> 
>     My ulterior motive with this patch is not even to address that old
>     concern, but to avoid having to exclude the code comments from the
>     upcoming master -> main rename (because those code comments talk about
>     git/git's main branch, which is called master).

Also a good reason.  Why modify code when you can just delete it?
-- 
brian m. carlson (he/him or they/them)
Houston, Texas, US

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

      reply	other threads:[~2020-11-07  1:23 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-07  1:12 [PATCH] tests: consolidate the `file_size` function into `test-lib-functions.sh` Johannes Schindelin via GitGitGadget
2020-11-07  1:23 ` brian m. carlson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201107012345.GD6252@camp.crustytoothpaste.net \
    --to=sandals@crustytoothpaste.net \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.