Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
>> But I am not sure if that warrants switching of the default.
>
> Hopefully either this thread or a re-roll of that series will convince
> you, is this something you'd like to see on list in the next few days or
> is it better to hold off until after the RC period?
You sent me a message, said that you wanted to change "cat-file" and
that you wanted my opinion on it, I gave a response, and in response
to that, you just repeated that you still want to change it, and I
did not see any new justification with it in the message I am
responding to, so sorry, but I fail to see where that "hopefully"
comes from.
The way I see it, if there is no practical difference, leaving the
existing behaviour of "cat-file" the same would be the most prudent
thing to do, especially when the primary goal of the topic is not
about improving "cat-file". This is merely a fallout from the work
to improve lower layers so that "fsck" can work better with objects
with unexpected types, no?
I suspect that this is one of the reasons your topics take way too
more time than they should to mature: biting too much at one time.
Surely it will be about the same effort for you to change the
default, while updating the lower layer that results in making it
easier to have either default. However, having to argue for and
justify the change of default of a command unrelated to the main
focus of the topic would make your series longer, and adds one more
thing on the plate of reviewers and future readers of the series
that they have to reason about. If you keep the topic more focused
to "fsck" and the improvement of the lower layer to support it, you
may still need to update the callers that share the same lower layer
("cat-file" in this case) but you would reduce the burden of
reviewers if you can label the change as "this adjust another caller
to the updated lower layer, but no end-user visible change in its
behaviour", as there is one fewer thing for them to have to think.
> I think it's a much better approach to just implicitly supply a
> OBJECT_INFO_ALLOW_UNKNOWN_TYPE at a high level (you just want the total
> disk use, who cares if there's one bad object there), as opposed to
> teaching rev-list and others about --allow-unknown-type.
I actually was hoping that the update to the lower layer functions
would be to stop dying when they see an unknown type (instead they
can leave a bit of information in object_info to signal that the
object is bogus---perhaps the *typep having a value outside the four
real types may be a good enough implementation for that signal) and
leave the policy decision to the callers. That is, there is no
option bit OBJECT_INFO_ALLOW_UNKNOWN_TYPE that is passed from the
caller to the lower layer; the caller can decide what to do with a
bogus object themselves, "cat-file" retains the current "we'd die
unless allow-unknown is passed" default to reduce the impact from
the entire series to reduce cognitive load from reviewers to make it
easier to review the series, and if you really feel like doing the
"while at it, cat-file requiring --allow does not make sense; if the
user wants the raw inflated content, just let them have it", do so
outside the series after the dust settles.
I dunno. I may be agreeing with your "it's a much better approach
to just implicitly supply allow-unknown-type at a higher level".
