On 22.12.22 06:40, Jeff Davis wrote:
On Sat, 2022-12-17 at 19:14 -0800, Jeff Davis wrote:
Attached is a new patch series. I think there are enough changes that
this has become more of a "rework" of the collation code rather than
just a refactoring. This is a continuation of some prior work[1][2]
in
a new thread given its new scope.

Here's version 5. There are a number of fixes, and better tests, and
it's passing in CI.

The libc hook support is still experimental, but what's working is
passing in CI, even on windows. The challenges with libc hook support
are:

  * It obviously doesn't replace all of libc, so the separation is not
as clean and there are a number of callers throughout the code that
don't necessarily care about specific collations.

  * libc relies on setlocale() / uselocale(), which is global state and
not as easy to track.

  * More platform issues (obviously) and harder to test.

I'm confused by this patch set.

It combines some refactoring that was previously posted with partial support for multiple ICU libraries with partial support for some new hooks. Shouldn't those be three separate threads? I think the multiple ICU libraries already does have a separate thread; how does this relate to that work? I don't know what the hooks are supposed to be for? What other locale libraries are you thinking about using this way? How can we asses whether these interfaces are sufficient for that? The refactoring patches don't look convincing just by looking at the numbers:

 3 files changed, 406 insertions(+), 247 deletions(-)
 6 files changed, 481 insertions(+), 150 deletions(-)
 12 files changed, 400 insertions(+), 323 deletions(-)

My sense is this is trying to do too many things at once, and those things are each not fully developed yet.



Reply via email to