You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by GitBox <gi...@apache.org> on 2019/05/16 07:46:40 UTC
[GitHub] [lucene-solr] thomaswoeckinger commented on issue #665: Fixes
SOLR-11841, SOLR-13331, SOLR-13347
thomaswoeckinger commented on issue #665: Fixes SOLR-11841, SOLR-13331, SOLR-13347
URL: https://github.com/apache/lucene-solr/pull/665#issuecomment-492957537
The files where formatted with settings generated by 'ant eclipse', so they
where wrong before, i can remove the final modifier which i personally
prefer.
Some additional inflammations about the issues:
The issues are related to each other due to the change from JavaBinCodec to
use UTF8CharSequence instead of String, which leads to a collection.remove
calls which are useless because their type does not match.
The issue was introduced in version 7.7.x and initially detected on our
side by integration tests. I want to make sure that this issues will not
occur again in the future so i added missing unit tests and refactored the
test base to allow easy exchange of the used codec.
The test written for SOLR-13331 shows up two additional issues described by
SOLR-13347 and SOLR-11841. Also some unexpected internal state where caused
by the EmbeddedSolrServer which i corrected to (return null stream instead
of empty).
So i will remove the final modifiers and push again!
Thx for your time!
Best,
Tom
Erick Erickson <no...@github.com> schrieb am Mi., 15. Mai 2019,
19:56:
> There are two things that might help with the arbitrary change issue:
>
> 1> On the reviewing side, In InteliJ, I can choose an option “ignore all
> whitespace”, I’m sure other tools have similar. Doesn’t help with final and
> the like of course.
>
> 2> Again in IntelliJ and (presumably) other tools I can set the autoformat
> to only reformat changed lines rather than entire files.
>
> FWIW,
> Erick
>
> > On May 15, 2019, at 6:49 AM, David Smiley <no...@github.com>
> wrote:
> >
> > It'd help to review your changes if you made fewer arbitrary changes,
> like adding 'final' and changing indentation of javadocs that were fine as
> they were.
> > Also, it'd help to summarize why 3 different issues are being fixed in
> one PR. Might be just fine but please add info/context to make reviewer's
> job either or you may not get a review at all.
> >
> > —
> > You are receiving this because you were mentioned.
> > Reply to this email directly, view it on GitHub, or mute the thread.
> >
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/lucene-solr/pull/665?email_source=notifications&email_token=ACKCGQE2FNGKHHJ6EP6WVA3PVRFFBA5CNFSM4HLKBL4KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVPOJZQ#issuecomment-492758246>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ACKCGQCSSE62WLOMMUEHKLLPVRFFBANCNFSM4HLKBL4A>
> .
>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org