Re: The moment you've all been waiting for PLEASE READ, Gradle builds will start failing on warnings on 9x!

[Erick Erickson]

re: checkstyle. Feel free to have at that debate ;)

> On Jun 23, 2020, at 6:51 PM, Michael Sokolov <msoko...@gmail.com> wrote:
> 
> +1 thanks huge step forward for code hygiene
> 
>  Maybe someday we will agree on a minimal check style enforcement too 😮 ...
> Sorry, too soon?
> 
> On Tue, Jun 23, 2020, 6:21 PM Anshum Gupta <ans...@anshumgupta.net> wrote:
> Thanks, Erick! This is awesome :)
> 
> On Tue, Jun 23, 2020 at 2:18 PM Erick Erickson <erickerick...@gmail.com> 
> wrote:
> As of my push a few minutes ago, Gradle compiling on 9x  WILL FAIL if there 
> are any warnings in the code. See LUCENE-9411. I’ve finally finished 
> suppressing over 8,000 warnings in Solr, so could check this in. Many thanks 
> to Dawid for helping me with the Gradle parts. The goal now is to not add any 
> _more_ SuppressWarnings if at all possible. I hope we can start taking the 
> suppressions out when we’re working on code, so when working on code please 
> consider removing some of them.
> 
> I was hoping that we could also fail ant builds, but there are some tricky 
> dependencies in third party code that weren’t easy to resolve in the ant 
> world due to licensing issues, if you’re interested in details, see the JIRA 
> or ping me on Slack. One consequence of this is that 8x will NOT fail on 
> warnings, neither will Ant builds on 9x. If someone wants to try working that 
> out, please feel free but I’m just really tired of banging my head against 
> that wall.
> 
> So please, Please, PLEASE start compiling 9x with Gradle or cover your ears 
> to keep from hearing me complain. And I’ve been taking lessons from my 3 1/2 
> year old grandson on doing that LOUDLY.
> 
> About SuppressWarnings. There were so many of them that there was no hope of 
> actually fixing the underlying causes in one go. I’ve enhanced the BadApples 
> report to start reporting on the number of SuppressWarnings in each file week 
> to week when they increase or decrease. I’ll be nudging people if the number 
> of SuppressWarnings starts going up, starting Monday. I can’t help but think 
> understanding generics will be improved by working through new warnings.
> 
> A couple of side notes for IntelliJ users (IDK about other IDEs, but I’d be 
> surprised if there weren’t similar capabilities):
> 
> - When you just open the project, Gradle is automatically configured. There’s 
> no need to execute the “gradlew idea” task.
> 
> - You can execute tasks in IntelliJ _really easily_ by clicking on them in 
> the gradle window, it’s on the extreme right. It seems much more robust than 
> trying the same thing in Ant.
> 
> -- The “assemble” task will bring up a convenient window showing errors 
> (including warnings) that you can click on and get right to the offending 
> code. “classes” and “testClasses” are also very useful tasks to execute in 
> this context.
> 
> - The “inspections” in IntelliJ point out a lot of things, but not anything 
> with SuppressWarnings. It may be worth coming to consensus on which 
> inspections are worth enabling. And perhaps distributing a configuration. For 
> instance, do we really care for inspections reporting “blah could be final”? 
> They’re highlighted in yellow in my setup, and I’ve done nothing special. 
> Spend some time looking at those when you’re working on code… the number of 
> “method may return null” inspections is scary. Have we’ve ever had the 
> released code generate an NPE or anything like that <snark/>.
> 
> - Please do NOT suppress the _inspections_ in IntelliJ. One of the choices 
> IntelliJ offers is to suppress an inspection, and it adds a 
> “suppressInspection” comment to the source code specific to IntelliJ. This is 
> different than Javas’s SuppressWarnings, and we shouldn’t include comments in 
> the code specific to a particular IDE.
> 
> - The motivation here is that we need all the help from the compiler we can 
> get when it comes to as large and complex a code base as Lucene/Solr. Yes, it 
> feels constraining. Yes, it means we won’t feel as productive because we have 
> to take time to address things we’ve been ignoring. The leap of faith is that 
> if we spend a bit of time up front, we can avoid having to spend a lot _more_ 
> time fixing errors later in the release cycle. The time it takes to fix a 
> problem goes up exponentially the farther down the cycle it’s caught. Fixing 
> something when developing may take T minutes. Some time later when test start 
> failing, it takes T*X. And when you consider community-wide implications of 
> releasing code, getting feedback from the field, filing a JIRA, trying to 
> reproduce the problem, checking the code, and pushing a fix, the cost of 
> fixing something after it’s released goes up enormously. I’m not saying that 
> addressing all the complaints something like IntelliJ’s inspections show will 
> magically make it unnecessary to make point releases, but avoiding just a few 
> is a win.  <rant/>
> 
> Erick
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
> For additional commands, e-mail: dev-h...@lucene.apache.org
> 
> 
> 
> -- 
> Anshum Gupta


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