I think we could include a standard template in the dev-support directory, just like the formatter? With IDE support the developers can reorg the imports easily so if we do not force an order, lots of the contents in a patch will be the reorder of imports...
Andrew Purtell <[email protected]> 于2019年4月5日周五 上午4:32写道： > 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 >
