On 03/11/2014 10:41 PM, Brad King wrote:
> On Tue, Mar 11, 2014 at 4:06 PM, Junio C Hamano <gits...@pobox.com> wrote:
>> I may be misremembering things, but your first sentence quoted above
>> was exactly my reaction while reviewing the original change, and I
>> might have even raised that as an issue myself, saying something
>> like "consistency across values is more important than type-saving
>> in a machine format".
> For reference, the original design discussion of the format was here:
> I do not recall this issue being raised before, but now that it has
> been raised I fully agree:
> In -z mode an empty <newvalue> should be treated as missing just as
> it is for <oldvalue>. This is obvious now in hindsight and I wish I
> had realized this at the time. Back then I went through a lot of
> iterations on the format and missed this simplification in the final
> version :(
It's not your fault; anybody could have reviewed your code at the time
(I most of all, because I have been so active in this area of the code).
> Moving forward:
> The "create" command rejects a zero <newvalue> so the change in
> question for that command is merely the wording of the error message
> and there is no compatibility issue.
> The "update" command supports a zero <newvalue> so that it can
> be used for all operations (create, update, delete, verify) with
> the proper combination of old and new values. The change in question
> makes an empty <newvalue> an error where it was previously treated
> as zero. (BTW, Michael, I do not see a test case for the new error
> in your series. Something like the patch below should work.)
>> I am not against deprecating and removing
>> the support for it in the longer term, though.
> As I reported in my above-linked response, I'm not depending on
> the old behavior myself. Also if one were to start seeing this
> error then generated input needs only trivial changes to avoid it.
> If we do want to preserve compatibility for others then perhaps an
> empty <newvalue> with -z should produce:
> warning: update $ref: missing <newvalue>, treating as zero
This last suggestion is what I am implementing for the re-roll (coming
shortly). Thanks for the discussion.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html