[jira] [Commented] (SOLR-14474) Fix remaining auxilliary class warnings in Solr
[ https://issues.apache.org/jira/browse/SOLR-14474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17117899#comment-17117899 ] ASF subversion and git services commented on SOLR-14474: Commit 07a9b5d1b0eb06adbb6993de2ee615a09609ac90 in lucene-solr's branch refs/heads/master from Erick Erickson [ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=07a9b5d ] SOLR-14474: Fix remaining auxilliary class warnings in Solr > Fix remaining auxilliary class warnings in Solr > --- > > 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 > Time Spent: 1h 40m > Remaining Estimate: 0h > > 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
[jira] [Commented] (SOLR-14474) Fix remaining auxilliary class warnings in Solr
[ https://issues.apache.org/jira/browse/SOLR-14474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17117898#comment-17117898 ] ASF subversion and git services commented on SOLR-14474: Commit 473185ad06801ad444d7377a531396b51c106591 in lucene-solr's branch refs/heads/branch_8x from Erick Erickson [ https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=473185a ] SOLR-14474: Fix remaining auxilliary class warnings in Solr > Fix remaining auxilliary class warnings in Solr > --- > > 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 > Time Spent: 1h 40m > Remaining Estimate: 0h > > 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
[jira] [Commented] (SOLR-14474) Fix remaining auxilliary class warnings in Solr
[ https://issues.apache.org/jira/browse/SOLR-14474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17115599#comment-17115599 ] Erick Erickson commented on SOLR-14474: --- New PR up, incorporates David's comments. Thanks! > Fix remaining auxilliary class warnings in Solr > --- > > 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 > Time Spent: 1h 20m > Remaining Estimate: 0h > > 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
[jira] [Commented] (SOLR-14474) Fix remaining auxilliary class warnings in Solr
[ https://issues.apache.org/jira/browse/SOLR-14474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17114918#comment-17114918 ] Erick Erickson commented on SOLR-14474: --- See the PR, I'll commit Tuesday unless there are objections. > Fix remaining auxilliary class warnings in Solr > --- > > 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 > Time Spent: 10m > Remaining Estimate: 0h > > 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
