It depends on the platform you are using, which is the bug. What is most important WRT compatibility is the API signature's binary compatibility which would be maintained.
As the code is now, a result value is effectively meaningless without knowing the "file.encoding" at the time the value was generated. There is no way to know this of course. Gary On Sun, Dec 29, 2019, 11:58 Claude Warren <cla...@xenei.com> wrote: > Changing the getBytes() call will change the operation vs the previous > release. That would be a major release change would it not? > > On Sun, Dec 29, 2019 at 4:20 PM Gary Gregory <garydgreg...@gmail.com> > wrote: > > > On Sun, Dec 29, 2019 at 10:58 AM Gary Gregory <garydgreg...@gmail.com> > > wrote: > > > > > The two murmur hash classes call String#getBytes() instead of > > > String#getBytes(Charset|String). > > > > > > This means you can get different results depending on where in the > world > > > you run the code or by changing the "file.encoding" system property. I > > > can't imagine that's the intention here. > > > > > > Why not use UTF-8? > > > > > > > Or ISO_8859_1... > > > > Gary > > > > > > > > Gary > > > > > > > > > On Sun, Dec 29, 2019 at 8:28 AM Gary Gregory <garydgreg...@gmail.com> > > > wrote: > > > > > >> On Sun, Dec 29, 2019 at 4:20 AM Alex Herbert < > alex.d.herb...@gmail.com> > > >> wrote: > > >> > > >>> On Sun, 29 Dec 2019, 01:14 Gary Gregory, <garydgreg...@gmail.com> > > wrote: > > >>> > > >>> > It looks like public methods have been removed > > >>> > from org.apache.commons.codec.digest.MurmurHash3$IncrementalHash32, > > >>> These > > >>> > need to go back in to maintain binary compatibility. Then we can > > have a > > >>> > release candidate. > > >>> > > > >>> > > >>> To fix the broken hash implementation a new parent class with the > > correct > > >>> implementation was introduced and some methods bumped up to that. So > > the > > >>> methods still exist but they are in the parent class. When I ran > clirr > > >>> during the development it did not complain. Is there a report stating > > >>> that > > >>> binary compatibility is broken? Maybe JApiCmp has a different > opinion. > > >>> > > >>> Doing it this way allows the broken class to be deprecated. The other > > way > > >>> to have the new correct class as the child means that the broken > class > > >>> cannot be deprecated as it has a child. Making the broken method > > >>> deprecated > > >>> would then have a child overriding a deprecated method. > > >>> > > >> > > >> You're correct, I misread the JApiCmp report. Sorry about that. > > >> > > >> Gary > > >> > > >> > > >>> > > >>> Alex > > >>> > > >>> > > >>> > Gary > > >>> > > > >>> > On Fri, Dec 27, 2019 at 7:02 PM Gary Gregory < > garydgreg...@gmail.com > > > > > >>> > wrote: > > >>> > > > >>> > > On Fri, Dec 27, 2019 at 3:18 PM Alex Herbert < > > >>> alex.d.herb...@gmail.com> > > >>> > > wrote: > > >>> > > > > >>> > >> > > >>> > >> > > >>> > >> > On 27 Dec 2019, at 16:35, Gary Gregory < > garydgreg...@gmail.com> > > >>> > wrote: > > >>> > >> > > > >>> > >> > Great, TY. Feel free to add more tests if need be. Edge cases > > and > > >>> so > > >>> > on. > > >>> > >> > > > >>> > >> > Gary > > >>> > >> > > >>> > >> If you look at the Jacoco report for MurmurHash3 the only line > > >>> missing > > >>> > >> execution is the throwing of an AssertionError in a default > block > > >>> of a > > >>> > >> switch statement for a line that should not be possible to reach > > >>> (line > > >>> > >> 1057). > > >>> > >> > > >>> > >> So it is missing coverage of unreachable code. > > >>> > >> > > >>> > >> This is part of the original code that I did not update. I can > > >>> rewrite > > >>> > it > > >>> > >> to drop the unreachable code but as it stands it is self > > >>> documenting. > > >>> > >> > > >>> > >> My preference would be to drop the unreachable code. It is not > > there > > >>> > >> because it needs to be, for example a catch block to handle a > > >>> declared > > >>> > >> exception that you never expect. It seems to be to add a default > > >>> block > > >>> > for > > >>> > >> the switch statement. > > >>> > >> > > >>> > > > > >>> > > I'm OK to drop the code, or replace the AssewrtionError with an > > >>> > > IllegalStateException? If any kind of code remains, the exception > > >>> message > > >>> > > and/or comment should state "this should not happen" but I can > > >>> imagine it > > >>> > > could if someone put this through some fuzzer. > > >>> > > > > >>> > > Gary > > >>> > > > > >>> > > > > >>> > >> WDYT? > > >>> > >> > > >>> > >> Alex > > >>> > >> > > >>> > >> > > >>> > >> > > > >>> > >> > On Fri, Dec 27, 2019 at 10:54 AM Alex Herbert < > > >>> > alex.d.herb...@gmail.com > > >>> > >> > > > >>> > >> > wrote: > > >>> > >> > > > >>> > >> >> I'll have a look at this since I rewrote the code and all the > > >>> tests > > >>> > to > > >>> > >> fix > > >>> > >> >> the hash implementation. > > >>> > >> >> > > >>> > >> >> Alex > > >>> > >> >> > > >>> > >> >> On Fri, 27 Dec 2019, 15:18 Gary Gregory, < > > garydgreg...@gmail.com > > >>> > > > >>> > >> wrote: > > >>> > >> >> > > >>> > >> >>> Hi Claude, > > >>> > >> >>> > > >>> > >> >>> Is there any way we could get 100% code coverage > > >>> > >> >>> on MurmurHash3$IncrementalHash32x86 ? There is a corner case > > >>> left > > >>> > >> >> untested. > > >>> > >> >>> > > >>> > >> >>> To see the code coverage, look at the JaCoCo report in > > >>> > >> >>> target\site\index.html under 'Project Reports' after running > > >>> 'mvn > > >>> > >> clean > > >>> > >> >>> package site -P jacoco -P japicmp' > > >>> > >> >>> > > >>> > >> >>> Gary > > >>> > >> >>> > > >>> > >> >>> On Thu, Dec 26, 2019 at 5:03 PM Claude Warren < > > cla...@xenei.com > > >>> > > > >>> > >> wrote: > > >>> > >> >>> > > >>> > >> >>>> For the contributions and issues I was involved in, the > > javadoc > > >>> > >> appear > > >>> > >> >> to > > >>> > >> >>>> be correct. > > >>> > >> >>>> > > >>> > >> >>>> Claude > > >>> > >> >>>> > > >>> > >> >>>> On Thu, Dec 26, 2019 at 1:30 PM Gary Gregory < > > >>> > garydgreg...@gmail.com > > >>> > >> > > > >>> > >> >>>> wrote: > > >>> > >> >>>> > > >>> > >> >>>>> It looks like we will need a new version of Commons Codec > > out > > >>> > before > > >>> > >> >> we > > >>> > >> >>>> can > > >>> > >> >>>>> use new code there from Commons Collections. So now's the > > >>> time to > > >>> > >> >>> polish, > > >>> > >> >>>>> PR, and so on. > > >>> > >> >>>>> > > >>> > >> >>>>> If you've contributed code to Codec, please make sure the > > >>> Javadoc > > >>> > >> are > > >>> > >> >>>>> helpful and the site up to date. > > >>> > >> >>>>> > > >>> > >> >>>>> Thank you! > > >>> > >> >>>>> Gary > > >>> > >> >>>>> > > >>> > >> >>>> > > >>> > >> >>>> > > >>> > >> >>>> -- > > >>> > >> >>>> I like: Like Like - The likeliest place on the web > > >>> > >> >>>> <http://like-like.xenei.com> > > >>> > >> >>>> LinkedIn: http://www.linkedin.com/in/claudewarren > > >>> > >> >>>> > > >>> > >> >>> > > >>> > >> >> > > >>> > >> > > >>> > >> > > >>> > >> > > >>> --------------------------------------------------------------------- > > >>> > >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > >>> > >> For additional commands, e-mail: dev-h...@commons.apache.org > > >>> > >> > > >>> > >> > > >>> > > > >>> > > >> > > > > > -- > I like: Like Like - The likeliest place on the web > <http://like-like.xenei.com> > LinkedIn: http://www.linkedin.com/in/claudewarren >
