Cool, will get those in and backported. Thanks, Ankit! On Wed, Apr 3, 2019 at 4:00 PM Ankit Singhal <[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." > > > 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 > > > -- Best regards, Andrew Words like orphans lost among the crosstalk, meaning torn from truth's decrepit hands - A23, Crosstalk
