"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."
I have also gone through the same pain. After fixing the eclipse formatter and import order, a pre-commit job is now little nicer to me. :) HBASE-20912 Add import order config in dev support for eclipse HBASE-20911 Incorrect Swtich/case indentation in formatter template for eclipse Regards, Ankit Singhal On Wed, Apr 3, 2019 at 2:15 PM 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 >
