Hello,
On Thu, March 9, 2017 9:04 am, Alexander Korotkov wrote:
> On Wed, Feb 1, 2017 at 2:43 PM, Emre Hasegeli <emre@hasegeli.com> wrote:
>
>> > I think this patch is already in a good shape.
>>
>> I am sorry for introducing this bug. This fix looks good to me as well.
>
>
> I checked this patch too. And it seems good to me as well.
> Should we mark it as "ready for committer"?
I can't comment on the code, but the grammar on the comments caught my eye:
> +/* Can any range from range_box does not extend higher than this
argument? */
>
>+static bool
>+overLower2D(RangeBox *range_box, Range *query)
>+{
>+ return FPle(range_box->left.low, query->high) &&
>+ FPle(range_box->right.low, query->high);
>+}
The sentence sounds quite garbled in English. I'm not entirely sure what
it should be, but given the comment below "/* Can any range from range_box
to be higher than this argument? */" maybe something like:
/* Does any range from range_box extend to the right side of the query? */
If used, an analog wording should be used for overHigher2D's comment like:
/* Does any range from range_box extend to the left side of the query? */
Also:
/* Can any range from range_box to be higher than this argument? */
should be:
/* Can any range from range_box be higher than this argument? */
Another question: Does it make sense to add the "minimal bad example for
the '&<' case" as test case, too? After all, it should pass the test after
the patch.
Bets regards,
Tels
