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 <notificati...@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 <notificati...@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: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
