Review for Hunspell update
RyanVM
As far as I know, the spelling checker module has not had an owner
since Scott left. In a few instances in the past, Olli has stepped up
and done reviews, and as such, I've requested review from him again
for this release. However, I'm wondering whether or not it even needs
review from him? I've noticed that some other third party libraries
(sqlite to name one) do not require r+ for being updated in the tree,
other than any mozilla-specific changes that accompany it.
Is there a policy for which third party libraries need an r+ and which
ones don't? Does Hunspell need one? Are there any plans to name a new
owner for spellcheck? Please let me know as I would like to get this
update in the tree and on the branches due to it fixing a significant
number of bugs from the current 1.2.8 in-tree version.
Mike Connor
-- Mike
[1] http://www.sqlite.org/testing.html
> _______________________________________________
> dev-planning mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-planning
RyanVM
> >https://lists.mozilla.org/listinfo/dev-planning
Talking to the Hunspell devs, Hunspell has tests in-tree that are run
prior to every release, both with and without Valgrind running. I'm
working on getting more information about what is tested and how often
tests are added. Also keep in mind that Hunspell receives patches from
other projects that use it as well (such as OOo), so the code is being
reviewed by eyeballs outside the Mozilla project as well.
From where I'm coming from, I don't mind having Hunspell updates go
through a review before getting pushed into the tree if need be, but
in the absence of a clear spellcheck owner, it's like pulling teeth to
get updates in. It seems that I can't find anyone who understands the
code who can make time to do the review. Mike, I remember having a
similar conversation with you with respect to spellcheck not having an
owner when I was working on getting 1.2.8 landed (before 3.5.x even
branched!). It was challenging to say the least. It seems to me that
1.2.11 is going to be more of the same. I'm not faulting Olli as I
know he has a ton of other work on his plate, and frankly I'm not sure
it's very fair to him to even be putting this review on his shoulders.
But it seems I can't get any traction at all on getting code reviewed
and landed that fixes numerous existing bugs on file with the current
version, and that along with the continuing lack of a spellcheck owner
is very frustrating.
Mike Beltzner
> But it seems I can't get any traction at all on getting code reviewed
> and landed that fixes numerous existing bugs on file with the current
> version, and that along with the continuing lack of a spellcheck owner
> is very frustrating.
Yes, that is understandably frustrating. As I see things, mconnor is
trying to help determine if we can blanket r this code, isn't he?
cheers,
mike
Justin Wood (Callek)
After reading this thread I wonder if making RyanVM an owner of HunSpell
as it relates to us is meaningful. But I make no notion of having done
any vetting for this suggestion.
--
~Justin Wood (Callek)
Mike Connor
This sounds really promising. More detail here would be awesome. Also, perhaps we should also be running these tests as part of our test harness? Not sure what their platform coverage looks like, and I'm sure we'd all feel good about.
I think if there's robust test coverage, and someone (such as yourself?) is willing to track upstream Hunspell, it's likely worth trying a sqlite-like blanket approval for merges from upstream.
> From where I'm coming from, I don't mind having Hunspell updates go
> through a review before getting pushed into the tree if need be, but
> in the absence of a clear spellcheck owner, it's like pulling teeth to
> get updates in. It seems that I can't find anyone who understands the
> code who can make time to do the review.
I think the lack of an owner is a symptom of a larger problem, which is that there aren't enough people looking at that code to have effective/timely reviews. But that may be okay, if the code is usable as-is and isn't on us to maintain.
> Mike, I remember having a
> similar conversation with you with respect to spellcheck not having an
> owner when I was working on getting 1.2.8 landed (before 3.5.x even
> branched!).
Yeah, I was point on this, and I'm truly sorry that it's dragged out this long. Let's not let this stagnate again.
-- Mike
Axel Hecht
I've been glancing at the patch itself, and I don't think it's a version
that's safe to whitelist, FWIW. There's quite a bit of removal of
mozilla-specific code in that patch that should undergo discussion at
least, if not thorough review. Which means, I don't know why we
currently have it, nor why it's not the new updated hunspell drop.
Axel
chris hofmann
If/when it does land we should organize a around of international
testing to identify any bugs like this one that sat hidden for many
months after a previous landing.
https://bugzilla.mozilla.org/show_bug.cgi?id=525581
Do we have test cases for this particular problem and anything related yet?
--chofmann
Axel Hecht
That might put the burdon to check for compat with the dictionaries out
there off of our shoulders.
Axel
Giacomo Magnini
> Also, one thing I came up with, what version of hunspell does OOo ship?
>
According to this: http://development.openoffice.org/releases/DEV300m77_snapshot.html
they just started using 1.2.9 (in a dev build), well behind 1.2.11 which is the current version.
Ciao, Giacomo.