Thread
-
Re: Fix comments on _bt_skiparray_strat_increment/decrement
Yugo Nagata <nagata@sraoss.co.jp> — 2025-12-31T09:40:20Z
On Wed, 31 Dec 2025 15:18:06 +0800 Chao Li <li.evan.chao@gmail.com> wrote: > > > > On Dec 31, 2025, at 14:57, Yugo Nagata <nagata@sraoss.co.jp> wrote: > > > > On Wed, 31 Dec 2025 11:36:56 +0800 > > Chao Li <li.evan.chao@gmail.com> wrote: > > > >> > >> > >>> On Dec 30, 2025, at 18:01, Yugo Nagata <nagata@sraoss.co.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@sraoss.co.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@sraoss.co.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@sraoss.co.jp>