From: Jonathan Nieder <jrnieder@gmail.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/2] teach git-config to output large integers
Date: Tue, 20 Aug 2013 23:34:58 -0700 [thread overview]
Message-ID: <20130821063458.GB2802@elie.Belkin> (raw)
In-Reply-To: <20130821050053.GA875@sigill.intra.peff.net>
Jeff King wrote:
> On Tue, Aug 20, 2013 at 09:38:41PM -0700, Jonathan Nieder wrote:
>> That is what I was trying to get at in discussing the test. It is not
>> "We would like --int to reject values higher than this, but some
>> platforms do not allow us to", but "Either rejecting this value, or
>> even better, computing the right size and printing it, is an
>> acceptable behavior, and this test checks for those."
>
> You are conflating the two patches, I think. The test we were discussing
> is for the _first_ patch, which fixes a bug in the range check. It is
> not meant to test git-config in particular, but to test that values
> higher than INT_MAX and lower than LONG_MAX are properly range-checked.
>
> Forget the second patch for a moment. I believe the first one is a bug
> fix that we would want even if we do not take the second patch at all.
Sure. I'm not conflating the patches. What I mean is that tests are
supposed to test desirable behavior, whatever that is --- they are not
about preventing all behavior changes but only about preventing
regressions.
So talking about tests is a (perhaps overly roundabout) way to figure
out the desirable behavior.
In particular, at first glance I would think computing 3 * 2^20
instead of erroring out would be a *good* behavior, not a regression.
If that's right, it doesn't make sense to me to go to careful lengths
either to test that git continues to error out on most platforms, or
to introduce new options to ensure "git config --int" continues to
error out.
That is what I am trying to understand. Everything about the first
patch except for the test makes sense to me, but the test doesn't. As
you noted, we know the test won't pass on some platforms. Why is it
something we should *want* to pass?
Jonathan
next prev parent reply other threads:[~2013-08-21 6:35 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-20 22:39 [PATCH 0/2] git-config and large integers Jeff King
2013-08-20 22:42 ` [PATCH 1/2] config: properly range-check integer values Jeff King
2013-08-20 23:07 ` Jonathan Nieder
2013-08-21 2:55 ` Jeff King
2013-08-20 22:44 ` [PATCH 0/2] git-config and large integers Stefan Beller
2013-08-20 22:48 ` Jeff King
2013-08-20 22:47 ` [PATCH 2/2] teach git-config to output " Jeff King
2013-08-20 22:57 ` Jonathan Nieder
2013-08-21 3:00 ` Jeff King
2013-08-21 4:38 ` Jonathan Nieder
2013-08-21 5:00 ` Jeff King
2013-08-21 6:34 ` Jonathan Nieder [this message]
2013-08-20 23:06 ` [PATCH 0/2] git-config and " Junio C Hamano
2013-08-20 23:41 ` Junio C Hamano
2013-08-21 2:43 ` Jeff King
2013-08-21 2:34 ` Jeff King
2013-09-08 8:27 ` [PATCHv2 0/5] " Jeff King
2013-09-08 8:29 ` [PATCH 1/5] config: factor out integer parsing from range checks Jeff King
2013-09-08 8:33 ` [PATCH 2/5] config: properly range-check integer values Jeff King
2013-09-08 8:36 ` [PATCH 3/5] config: set errno in numeric git_parse_* functions Jeff King
2013-09-09 0:36 ` Eric Sunshine
2013-09-09 19:53 ` Jeff King
2013-09-08 8:38 ` [PATCH 4/5] config: make numeric parsing errors more clear Jeff King
2013-09-08 8:40 ` [PATCH 5/5] git-config: always treat --int as 64-bit internally Jeff King
2013-09-09 18:58 ` [PATCHv2 0/5] git-config and large integers Junio C Hamano
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130821063458.GB2802@elie.Belkin \
--to=jrnieder@gmail.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).