search

cmusphinx / sphinx4

Pure Java speech recognition library
cmusphinx.sourceforge.net
Other
1.4k stars 587 forks source link

Fixed 8 important bugs and two dozen minor bugs from findbugs #47

Closed DrDub closed 8 years ago

DrDub commented 8 years ago

The details of the bugs fixed are in the commit log.

Findbugs was executed in the sphinx4-core folder after a successful gradle build as follows:

sphinx4-core$ findbugs -textui -auxclasspath ~/.m2/repository/org/apache/commons/commons-math3/3.2/commons-math3-3.2.jar -sortByClass -high build/classes/main

There are still close to 100 "relying in default encoding" bugs. That might affect people running sphinx4 in environments where UTF-8 is not the default encoding.

mbait commented 8 years ago

I'd say the the whole diff is not easy to review, some moments are unclear. I'd rather split this diff into parts each of which fixes a single bug, so you could explain the rationale behind the change.

mbait commented 8 years ago

Oh, I just realized that you made it into the commit message. I'm very sorry, will read it now.

DrDub commented 8 years ago

I just followed findbugs comments. Feel free to reject any changes you feel don't make the code easier / understandable. There are a few bugs that are real bugs and it would be nice to see them fixed thoug. I'll be glad to go through them one by one (I had to do some reading to understand what was the bug findbugs was complaining about, I can share the links I got), just add another ping to your earlier comments and we can discuss. I can then "water down" the PR by making fewer changes.

mbait commented 8 years ago

Thanks for the work, it's highly appreciated! I accepted and committed the most of changes. Unfortunately, I couldn't use automatic merge as the main work is still being done on SourceForge, so the only credit I could give to you was mention in the commit message.