If you want to try it out, I pushed a branch to my Geode repo that contains this change: https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin <https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin>
> On Oct 12, 2016, at 2:27 PM, Darrel Schneider <[email protected]> wrote: > > I like Dan's idea of catching formatting issues earlier but I think adding > 5-10 minutes to "build" would be too much. Currently when I'm trying to do > a quick build I use -xjavadoc. I'd probably do the same for this target if > it was part of build until I'm ready to do a precheckin. > > Mark, wouldn't running the formatter on all our java files and checking > them in get these issues down to 0? > > On Wed, Oct 12, 2016 at 12:53 PM, Udo Kohlmeyer <[email protected]> > wrote: > >> +1 - adding checkstyle to precheckin. >> >> If the developer uses the provided templates ( eclipse + intellij) then >> most of the formatting issues should be handled before precheckin. Also, if >> a developer has a questionable coding style, that should lessen as that >> developer will have resolve the issues before being able to commit. >> >> I also believe that this should not be an overbearing and intrusive >> process. >> >> --Udo >> >> >> >> On 13/10/16 6:36 am, Mark Bretl wrote: >> >>> Dan, >>> >>> There is some extra amount of time, 5-10 minutes extra for the entire >>> project (depending on your CPU). I think the real issue to adding it to >>> the >>> precheckin target and have it be 'effective' is it needs to run >>> successfully, otherwise it would turn into noise most of the time I think. >>> We need to get the issues down to 0 or manage to set a new baseline (not >>> the best idea), which is a lot of work, to make it run successfully. Right >>> now, if you run the target, it will fail every time since there >>> outstanding >>> issues in the code and very hard to tell what issues were introduced. >>> >>> >>> --Mark >>> >>> On Wed, Oct 12, 2016 at 11:34 AM, Dan Smith <[email protected]> wrote: >>> >>> Seems like it should run as part of the build target. The only reason to >>>> make it part of precheckin is if it takes a long time, otherwise people >>>> should get fast feedback they need to change their code before they push. >>>> >>>> -Dan >>>> >>>> On Wed, Oct 12, 2016 at 11:24 AM, Jared Stewart <[email protected]> >>>> wrote: >>>> >>>> +1 to running during the precheckin target as well as Travis CI >>>>> >>>>> On Oct 12, 2016 11:20 AM, "Darrel Schneider" <[email protected]> >>>>> wrote: >>>>> >>>>> If Travis CI is only run on pull requests then that is not enough >>>>>> >>>>> because >>>> >>>>> committers do not submit pull requests. Having it run during the gradle >>>>>> build or precheckin target is also needed. In addition to that I also >>>>>> wanted PRs to be checked. >>>>>> >>>>>> >>>>>> On Wed, Oct 12, 2016 at 11:12 AM, Jared Stewart <[email protected]> >>>>>> wrote: >>>>>> >>>>>> It would certainly be necessary to make sure that the code style to >>>>>>> >>>>>> be >>>> >>>>> enforced is sensible, e.g. doe not use wildcard imports. We would >>>>>>> >>>>>> also >>>> >>>>> want to make one large commit to format all existing code before >>>>>>> >>>>>> turning >>>>> >>>>>> this on. >>>>>>> >>>>>>> Mark - Thank you for the information about the existing setup. >>>>>>> >>>>>>> Mark, Darrel, Kevin - Given all of your comments, I think it might >>>>>>> >>>>>> make >>>> >>>>> more sense to add the flag to enable it in Travis CI rather than as >>>>>>> >>>>>> part >>>>> >>>>>> of the build. This way your build pass regardless of whitespace, >>>>>>> >>>>>> but >>>> >>>>> the >>>>>> >>>>>>> CI job would fail on PRs if they did not adhere to the standard >>>>>>> >>>>>> formatting. >>>>>> >>>>>>> Anthony - It doesn’t seem to me that turning this on would have the >>>>>>> >>>>>> effect >>>>>> >>>>>>> of combining reformatting commits and logic changes. Rather, since >>>>>>> >>>>>> all >>>> >>>>> code would already be formatted, there would no longer be any >>>>>>> >>>>>> reformatting >>>>>> >>>>>>> commits except for single large commits when the code style file was >>>>>>> updated. >>>>>>> >>>>>>> On Oct 12, 2016, at 11:01 AM, Bruce Schuchardt < >>>>>>>> >>>>>>> [email protected] >>>>> >>>>>> wrote: >>>>>>> >>>>>>>> I like the idea of doing this but I don't think Checkstyle should >>>>>>>> >>>>>>> be >>>> >>>>> enabled until all of the code is reformatted. >>>>>>> >>>>>>>> Also, last time I checked there was still a problem with the >>>>>>>> >>>>>>> IntelliJ >>>> >>>>> auto-format settings. It still used wildcard imports, which I >>>>>>> >>>>>> believe >>>> >>>>> we >>>>> >>>>>> don't allow. I've manually changed my settings in Editor->Code >>>>>>> >>>>>> Style->Java >>>>>> >>>>>>> to "Use single class import" to correct that problem. I couldn't see >>>>>>> >>>>>> how >>>>> >>>>>> to get Gradle to do it. >>>>>>> >>>>>>>> >>>>>>>> Le 10/12/2016 à 10:28 AM, Anthony Baker a écrit : >>>>>>>> >>>>>>>>> Source code with a consistent look-and-feel makes it easier for >>>>>>>>> >>>>>>>> people >>>>> >>>>>> to join the project community and contribute. >>>>>>> >>>>>>>> Let’s continue to keep reformatting commits separate from logic >>>>>>>>> >>>>>>>> changes—otherwise it’s too hard to review. >>>>>>> >>>>>>>> Anthony >>>>>>>>> >>>>>>>>> On Oct 12, 2016, at 10:06 AM, Dan Smith <[email protected]> >>>>>>>>>> >>>>>>>>> wrote: >>>> >>>>> +1 >>>>>>>>>> >>>>>>>>>> This might be a good time to reformat the code since I don't >>>>>>>>>> >>>>>>>>> think >>>> >>>>> there >>>>>>> >>>>>>>> are too many long lived feature branches outstanding. >>>>>>>>>> >>>>>>>>>> -Dan >>>>>>>>>> >>>>>>>>>> On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart < >>>>>>>>>> >>>>>>>>> [email protected] >>>>> >>>>>> wrote: >>>>>>> >>>>>>>> I would like to advocate for adding a Checkstyle < >>>>>>>>>>> >>>>>>>>>> http://checkstyle >>>>> >>>>>> . >>>>>> >>>>>>> sourceforge.net/> or Spotless <https://github.com/diffplug/ >>>>>>>>>>> >>>>>>>>>> spotless >>>>> >>>>>> gradle task to our build process to ensure that all code checked >>>>>>>>>>> >>>>>>>>>> in >>>>> >>>>>> meets >>>>>>> >>>>>>>> the formatting standards described on the wiki < >>>>>>>>>>> >>>>>>>>>> https://cwiki.apache.org/ >>>>>>> >>>>>>>> confluence/display/GEODE/Code+Style+Guide> (and in the >>>>>>>>>>> >>>>>>>>>> intellij/eclipse >>>>>>> >>>>>>>> formatter xml files in our repository). This will alleviate >>>>>>>>>>> >>>>>>>>>> difficulties >>>>>>> >>>>>>>> reviewing code when whitespace or formatting has changed since >>>>>>>>>>> >>>>>>>>>> all >>>> >>>>> code >>>>>>> >>>>>>>> checked in will already comply with standards. >>>>>>>>>>> >>>>>>>>>> >>>>>>> >>
