[ 
https://issues.apache.org/jira/browse/SOLR-14474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17106288#comment-17106288
 ] 

Erick Erickson commented on SOLR-14474:
---------------------------------------

Here's a trick. The default settings stop issuing warnings at 100. Let's say 
you have a class you're working on that is not shown because it has warnings 
230 - 250. Change gradle/defaults-java.gradle and add *-Xmaxwarns", "10000"*, 
to the *options.compilerArgs += [* section. Then you can search the output 
window (at least in IntelliJ) for your class and click on the warnings and be 
taken to the offending line of code. I usually work backwards so the line 
numbers match up....

Don't check that change to defaults-java.gradle in of course, although it would 
just produce lots of output...

> Fix auxilliary class warnings in Solr core
> ------------------------------------------
>
>                 Key: SOLR-14474
>                 URL: https://issues.apache.org/jira/browse/SOLR-14474
>             Project: Solr
>          Issue Type: Sub-task
>            Reporter: Erick Erickson
>            Assignee: Erick Erickson
>            Priority: Major
>
> We have quite a number of situations where multiple classes are declared in a 
> single source file, which is a poor practice. I ran across a bunch of these 
> in solr/core, and [~mdrob] fixed some of these in SOLR-14426. [~dsmiley] 
> looked at those and thought that it would have been better to just move a 
> particular class to its own file. And [~uschindler] do you have any comments?
> I have a fork with a _bunch_ of changes to get warnings out that include 
> moving more than a few classes into static inner classes, including the one 
> Mike did. I do NOT intend to commit this, it's too big/sprawling, but it does 
> serve to show a variety of situations. See: 
> https://github.com/ErickErickson/lucene-solr/tree/jira/SOLR-10810 for how 
> ugly it all looks. I intend to break this wodge down into smaller tasks and 
> start over now that I have a clue as to the scope. And do ignore the generics 
> changes as well as the consequences of upgrading apache commons CLI, those 
> need to be their own JIRA.
> What I'd like to do is agree on some guidelines for when to move classes to 
> their own file and when to move them to static inner classes.
> Some things I saw, reference the fork for the changes (again, I won't check 
> that in).
> 1> DocValuesAcc has no fewer than 9 classes that could be moved inside the 
> main class. But they all become "static abstract". And take 
> "DoubleSortedNumericDVAcc" in that class, It gets extended over in 4 other 
> files. How would all that get resolved? How many of them would people 
> recommend moving into their own files? Do we want to proliferate all those? 
> And so on with all the other plethora of classes in 
> org.apache.solr.search.facet.
> This is particularly thorny because the choices would be about a zillion new 
> classes or about a zillion edits.
> Does the idea of abstract .vs. concrete classes make any difference? IOW, if 
> we change an abstract class to a nested class, then maybe we just have to 
> change the class(es) that extend it?
> 2> StatsComponent.StatsInfo probably should be its own file?
> 3> FloatCmp, LongCmp, DoubleCmp all declare classes with "Comp" rather than 
> "Cmp". Those files should just be renamed.
> 4> JSONResponseWriter. ???
> 5> FacetRangeProcessor seems like it needs its own class
> 6> FacetRequestSorted seems like it needs its own class
> 7> FacetModule
> So what I'd like going forward is to agree on some guidelines to resolve 
> whether to move a class to its own file or make it nested (probably static). 
> Not hard-and-fast rules, just something to cut down on the rework due to 
> objections.
> And what about backporting to 8x? My suggestion is to backport what's 
> easy/doesn't break back-compat in order to make keeping the two branches in 
> sync.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to