Teng Long

From: Teng Long <dyroneteng@gmail.com>
Date: Mar 29, 2022, 07:38 UTC
Message-ID: <20220329073851.80702-1-dyroneteng@gmail.com>
In-Reply-To: <220324.867d8jo45p.gmgdl@evledraar.gmail.com>

> Was there an on-list v0 (RFC?) or is this a range-diff against nothing?
> Best not to include it until a v2 then.

My careless, there's no RFC, actually this patch should begin with v0 and without range-diff.

> Sometimes it's better to split up patches, but I think these 3x should
> really be squashed together. We make incremental progress to nowhere in
> 1/3 and 2/3, and it all comes together in 3/3. The 1-2/3 are trivial
> enough that we can squash them in.

Sure.

>	+	trace2_data_string("midx", r, "core.multipackIndex",
>	+						   r->settings.core_multi_pack_index ? "true" : "false");
>
> Weird indentation here.

Will fix.

> I.e. surely you just want to create a region, and if you care enough to
> log failure shouldn't we log that in open_midx_bitmap_1() if we care,

Actually, I just want to use "trace2_data_string()" at first because which informations I want to append is not so many, or does "create a region" a preferred principle for using TRACE2 APIs?

> and perhaps error() there instead of silently returning -1?

I think so, after I checked function "error_builtin()" and there is a "trace2_cmd_error_va()" usage in it which is already support to print some debug infos using TRACE2 envs.

Thanks.