Could we disable ImportOrder? This is the only one that seems to be particularly confusing and import order is a minor concern.
On Wed, Apr 3, 2019 at 10:08 PM 张铎(Duo Zhang) <[email protected]> wrote: > Yes, we could do this but it requires more magics in the personality > script... > > Will have a try later when we finish the github integration... > > Sean Busbey <[email protected]> 于2019年4月4日周四 上午8:40写道： > > > We could break out running with error prone into its own test like we do > > for building the website. > > > > On Wed, Apr 3, 2019, 19:16 张铎(Duo Zhang) <[email protected]> wrote: > > > > > Oh I forget that the error prone warnings are mixed with the javac > > > warnings, so the only way is to remove the -PerrorProne when compiling > in > > > pre commit if we do not want to be confused by the output. > > > > > > Or another hardcore solution is to fix all the error prone warnings... > > IIRC > > > we have about 1300+ warnings... > > > > > > Kevin Risden <[email protected]> 于2019年4月4日周四 上午7:26写道： > > > > > > > Assuming yes - Downloaded build artifacts [1] and ran "grep -rnF > > > > hbase_javac_logfilter ." checking for the new function name from > > > > HBASE-22100. > > > > > > > > grep -rnF hbase_javac_logfilter . > > > > ./patchprocess/precommit/personality/provided.sh:719:function > > > > hbase_javac_logfilter > > > > > > > > [1] > > https://builds.apache.org/job/PreCommit-HBASE-Build/16578/artifact/ > > > > > > > > Kevin Risden > > > > > > > > > > > > On Wed, Apr 3, 2019 at 7:17 PM Sean Busbey <[email protected]> > wrote: > > > > > > > > > Can anyone tell quickly if the error prone error is before or after > > > > > > > > > > https://issues.apache.org/jira/browse/HBASE-22100 > > > > > > > > > > On Wed, Apr 3, 2019, 16:15 Andrew Purtell <[email protected]> > > wrote: > > > > > > > > > > > Regarding the error-prone issues I am talking about, here is one > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://builds.apache.org/job/PreCommit-HBASE-Build/16578/artifact/patchprocess/diff-compile-javac-root.txt > > > > > > > > > > > > [WARNING] > > > > > > > > > > > > > > > > > > > > > /testptch/hbase/hbase-server/src/test/java/org/apache/hadoop/hbase/master/AbstractTestDLS.java:[654,54] > > > > > > [UnusedVariable] The parameter 'master' is never read. > > > > > > > > > > > > The submitted patch does not touch this file AbstractTestDLS and > > this > > > > > javac > > > > > > warning has the format of an error-prone finding. > > > > > > > > > > > > > > > > > > On Wed, Apr 3, 2019 at 2:11 PM Andrew Purtell < > [email protected] > > > > > > > > wrote: > > > > > > > > > > > > > I use Eclipse. Eclipse orders imports automatically per > formatter > > > > > > > settings. I have installed the HBase formatter from our > > dev-support > > > > > into > > > > > > > all of the relevant workspaces. This sometimes fails to do the > > > right > > > > > > thing > > > > > > > as far as checkstyle reporting in precommit is concerned. I > have > > > > tried > > > > > > > moving the indicated imports around by hand when this happens > and > > > it > > > > > > still > > > > > > > complains. I should not be required to use another IDE. (I > > won't.) > > > > > > Perhaps > > > > > > > the Eclipse formatter definition we ship in the project needs > an > > > > > update. > > > > > > > (From a contributor POV this shouldn't be necessary.) It's not > > > like I > > > > > am > > > > > > > trying to get away with being sloppy. > > > > > > > > > > > > > > HBASE-15560 is one. > > > > > > > HBASE-22114 amplifies it by having I think some unfortunate > > > > > interactions > > > > > > > between how Yetus decides what has changed and how to test it > and > > > > what > > > > > is > > > > > > > being attempted. > > > > > > > > > > > > > > On Wed, Apr 3, 2019 at 1:27 PM Josh Elser <[email protected]> > > > wrote: > > > > > > > > > > > > > >> Yeah, can you share some evidence of what you've been running > > > into, > > > > > > >> Andrew? > > > > > > >> > > > > > > >> The nit-picky tools are always a pain in the rear (especially > > when > > > > > > >> working across other branches) -- agree with you there. Can we > > > help > > > > > > >> lessen the pain by making it more clear what to run/inspect > when > > > QA > > > > > > >> reports something other than a +1? > > > > > > >> > > > > > > >> e.g. "I see you had some checkstyle issues. Please fix them > and > > > you > > > > > can > > > > > > >> re-verify locally by running `mvn ...`" > > > > > > >> > > > > > > >> I think, long run, these tools are nice to push towards > > > consistency > > > > > on. > > > > > > >> Wondering if there's more we can do to make working with them > > > easier > > > > > > >> before backtracking. > > > > > > >> > > > > > > >> On 4/3/19 4:05 PM, Xu Cang wrote: > > > > > > >> > "I think we need to revert the recent error-prone related > > work" > > > > > > >> > -- Andrew, can you please give an example about this? Such > as > > a > > > > JIRA > > > > > > >> that > > > > > > >> > has this kind of failure in pre-commit build. > > > > > > >> > > > > > > > >> > " Checkstyle's > > > > > > >> > ImportOrder is one that always trips me up and no matter > > where I > > > > > place > > > > > > >> the > > > > > > >> > imports continues to complain." > > > > > > >> > > > > > > > >> > -- I had some struggles about this too, though, after I > > > installed > > > > > > >> > checkstyle plugin in intellij and used it before generating > > > > patches, > > > > > > it > > > > > > >> > became less painful. I don't have any objection removing the > > > check > > > > > at > > > > > > >> the > > > > > > >> > same time. Contributors should still try their best to > > organize > > > > > > imports > > > > > > >> > cleanly and orderly. > > > > > > >> > > > > > > > >> > > > > > > > >> > "" > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > > >> > On Wed, Apr 3, 2019 at 12:02 PM Andrew Purtell < > > > > [email protected] > > > > > > > > > > > > >> wrote: > > > > > > >> > > > > > > > >> >> I have been contributing to this project for more than ten > > > years > > > > > and > > > > > > >> have > > > > > > >> >> noticed it is increasingly difficult to do so. > > > > > > >> >> > > > > > > >> >> For me the issues come down to precommit results. Precommit > > is > > > a > > > > > very > > > > > > >> >> useful tool, but *only if committers are attentive to > fixing > > > > > breaking > > > > > > >> >> changes immediately*. This has been an eternal problem. > > > > > > >> >> > > > > > > >> >> Also in fairness some problems I've thought are external to > > my > > > > > patch > > > > > > >> have > > > > > > >> >> turned out to be indirect consequences. Here the issue is > I'm > > > not > > > > > > able > > > > > > >> to > > > > > > >> >> trust precommit so true positive results are still > sometimes > > > > > suspect. > > > > > > >> >> > > > > > > >> >> I think we need to revert the recent error-prone related > > work, > > > > this > > > > > > >> seems > > > > > > >> >> to be the cause of some of the false failures in precommit > > jobs > > > > > I've > > > > > > >> looked > > > > > > >> >> at. > > > > > > >> >> > > > > > > >> >> In other cases we should adjust some static check settings. > > > > > > >> Checkstyle's > > > > > > >> >> ImportOrder is one that always trips me up and no matter > > where > > > I > > > > > > place > > > > > > >> the > > > > > > >> >> imports continues to complain. I'm at a loss and it's > really > > a > > > > > > trivial > > > > > > >> >> matter. Let's just turn it off. > > > > > > >> >> > > > > > > >> >> The transient issues we sometimes face with Apache build > > infra > > > > are > > > > > > >> possibly > > > > > > >> >> tolerable, I'm not referring to those. > > > > > > >> >> > > > > > > >> >> -- > > > > > > >> >> Best regards, > > > > > > >> >> Andrew > > > > > > >> >> > > > > > > >> >> Words like orphans lost among the crosstalk, meaning torn > > from > > > > > > truth's > > > > > > >> >> decrepit hands > > > > > > >> >> - A23, Crosstalk > > > > > > >> >> > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > Best regards, > > > > > > > Andrew > > > > > > > > > > > > > > Words like orphans lost among the crosstalk, meaning torn from > > > > truth's > > > > > > > decrepit hands > > > > > > > - A23, Crosstalk > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Best regards, > > > > > > Andrew > > > > > > > > > > > > Words like orphans lost among the crosstalk, meaning torn from > > > truth's > > > > > > decrepit hands > > > > > > - A23, Crosstalk > > > > > > > > > > > > > > > > > > > > > -- Best regards, Andrew Words like orphans lost among the crosstalk, meaning torn from truth's decrepit hands - A23, Crosstalk
