Hi,

> > The patch looks basically good to me. I've made some changes to the
> > regression test part as I want to have round-trip tests. I've merged
> > the tests checking the sortability to the existing tests and added
> > round-trip tests. With this change, we can test round-trip tests and
> > sortability tests with random UUID value in every test run while
> > minimizing the test time. Feedback is very welcome.

v11 looks good to me.

> It looks like leading, trailing, and embedded whitespace are all ignored. But 
> I don’t see a test case covering this behavior, so maybe it would be good to 
> add one.

I intentionally didn't include this test because the code is trivial:

``
        /* Skip whitespace */
        if (c == ' ' || c == '\t' || c == '\n' || c == '\r')
            continue;
```

And also because we never tested it for base64. If we want to start
testing it we should add tests both for base64 and base32hex which IMO
should be a separate patch.

-- 
Best regards,
Aleksander Alekseev


Reply via email to