Re: Fix comments on _bt_skiparray_strat_increment/decrement

On Wed, 31 Dec 2025 15:18:06 +0800
Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:

>
>
> > On Dec 31, 2025, at 14:57, Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:
> >
> > On Wed, 31 Dec 2025 11:36:56 +0800
> > Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> >
> >>
> >>
> >>> On Dec 30, 2025, at 18:01, Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:
> >>>
> >>> Hi,
> >>>
> >>> The attached patch is a trivial change to fix the comments on
> >>> _bt_skiparray_strat_increment() and _bt_skiparray_strat_decrement() so
> >>> that they are consistent with the comments on _bt_skiparray_strat_adjust().
> >>> At least the comment on _bt_skiparray_strat_decrement() containts an
> >>> obvious typo, since it mentions converting the high_compare key instead of
> >>> the low_compare key.
> >>>
> >>> Regards,
> >>> Yugo Nagata
> >>>
> >>> --
> >>> Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>
> >>> <fix_comments_on_bt_skiparray_strat_dec_inc_func.patch>
> >>
> >> Good catch. Looks like a copy/paste mistake.
> >>
> >> The code snippet prove the 2 functions' header comments are wrong:
> >
> > Thank you for your reviewing.
> >
> >> I also think we can delete “a” from the header comments. “into a >= key”, where “a” is an article (meaning one), but can be easily read as a variable name. The code comments don’t use “a” after “into”.
> >
> > The existing comments are grammatically correct, but as you point out,
> > removing the "a" might make them less confusing. However, the comments in
> > _bt_skiparray_strat_adjust() and in _bt_preprocess_array_keys_final(), which
> > call this function, also use "a" after "into".
> >
> > If we remove the "a" here, should we also update those comments for consistency?
> >
> > --
> > Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>
>
> It’s probably better to leave them as-is and just fix the error, which should make the patch easier to get through.

Agreed

Regards,
Yugo Nagata

--
Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>