Re: [RFC/PATCH] WIP: add deprecation & experimental process/interface

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, May 29, 2017 at 3:09 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
>> index 4f94fc7574..c76bbedf86 100755
>> --- a/GIT-VERSION-GEN
>> +++ b/GIT-VERSION-GEN
>> @@ -37,4 +37,5 @@ fi
>>  test "$VN" = "$VC" || {
>>       echo >&2 "GIT_VERSION = $VN"
>>       echo "GIT_VERSION = $VN" >$GVF
>> +     echo "GIT_VERSION_INT = $(echo $VN | sed -e 's/^\([0-9]*\)\.\([0-9]*\)\..*/\1\2/')" >>$GVF
>>  }
>
> Unlike Perl's v1.2.3.4 notation, this forces us worry when we go
> from v2.99.0 to v2.100.0 and eventually to v3.0, no?

Yeah it's just a dirty hack to get that WIP working, although at this
rate it'll take us ~20 years to reach 3.0 if we go up to 99, and this
would purely be internal to the codebase.

I think it make sense for core.version to be e.g. 2.13, and parsed
internally to 2013, then we have room to go to 2.999 or ~200 years at
the current dev pace.

>> +     } else if (1) {
>> +             /*
>> +              * TODO: Instead of `if 1` we should check a
>> +              * core.version variable here.
>> +              *
>> +              * I.e. if set to core.version=2.13 the user is opting
>> +              * in to get deprecations set at dep_at right away,
>> +              * and also perhaps experimental features from a
>> +              * sister experimental() interface.
>> +              */
>
> This essentially forces us to always read _some_ configuration.
> Some commands are meant to work outside repositories, so those who
> want to affect them needs to write core.version in their global
> configuration.  Some low-level plumbing commands may want to do
> absolute minimum without configurablity.

Doesn't making sure that those codepaths just don't call
experimental() or deprecate() solve that issue? Presumably if
something is such low-level plumbing that it can't call deprecate() or
experimental() we'd just create a new incompatible command under a
different name if we'd like to change it.

Or are there some edge cases I'm missing?

> I am not saying that it is absolutely a bad design decision to force
> us to read some configuration (yet); it's just that it is something
> that we have to keep in mind and always think about the
> ramifications of.

*Nod*. It's definitely a bit of a chicken & egg problem, especially if
we ever wanted to have experimental or deprecated config-parsing
directives, but for most parts of the codebase it should be fine.

>> +             die(_("Early bird deprecation error: %s"), message);
>> +     }
>> +}