Ilan: Failing on newly introduced warnings is much more draconian than having to run “gradlew check -x test” (or precommit). Compilation will fail. So I don’t think there’s any reason to add a note, it’ll smack you right in the face.
“gradle check -x test” is preferred now to “gradle precommit". “gradle check” by itself will do both precommit+ and testing all in one go. “gradle helpAnt” will show you most equivalent tasks to what we’re all familiar with from ant for retraining purposes ;). The Gradle precommit task was put there for “back compat”. Since “check” depends on “precommit”, check may contain, now or in future, more than precommit. IDK whether they’re identical at this point, Dawid put in that magic. Hmmm we might want to add a note that “gradlew check” does both. I think I might add that in the help text for gradle though.. precommit has never really been optional, or at least people who don’t run it and check in code that fails precommit were expected to fix it immediately. And _all_ of us have done that at one point or another. I have to say that Gradle is much faster, and just being to do “gradle check” and go do something else for a while has made it much more likely that I’ll run it more often. Erick > On Jun 24, 2020, at 2:32 PM, Ilan Ginzburg <ilans...@gmail.com> wrote: > > Thank you Erick! This is useful and saves time (I was able to set up gradle > with the assistance you gave me a while ago). > > I guess that also means Gradle precommit is no longer optional and likely the > text initializing PR's descriptions should mention that in some way... > > On Wed, Jun 24, 2020 at 9:13 AM Atri Sharma <a...@apache.org> wrote: > Thank you so much, Erick! > > On Wed, Jun 24, 2020 at 2:48 AM 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 > > > > > -- > Regards, > > Atri > Apache Concerted > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org > For additional commands, e-mail: dev-h...@lucene.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
