The task is fully suppressible with -x spotlessCheck. Also, if you have any formatter errors you can automatically fix them with 'gradle spotlessApply’.
> On Oct 13, 2016, at 9:40 AM, Kevin Duling <kdul...@pivotal.io> wrote: > > If we made formatting a warning, then people would probably quickly ignore > it. > If we made formatting an error, we need to be sure we don't get in to the > situation where <editor of choice>'s formatter is not in agreement with the > build's checker. > > I can live with an additional 17 seconds as well. And Jared's already > reduced the build time locally by 50%. But I still want the ability to > suppress the check similar to -x javadoc. > > On Wed, Oct 12, 2016 at 9:58 PM, William Markito <wmark...@pivotal.io> > wrote: > >> This sounds really good to me as well. +1 >> >> On Wed, Oct 12, 2016 at 4:13 PM, Jared Stewart <jstew...@pivotal.io> >> wrote: >> >>> This is running locally on my laptop. Since Spotless is only doing code >>> formatting and not any other static analysis, it already has 0 errors. >>> (Other than, of course, formatting not consistent with the template.) >>> >>>> On Oct 12, 2016, at 4:11 PM, Kenneth Howe <kh...@pivotal.io> wrote: >>>> >>>> Agree with Mark, this has to work with 0 errors before it would be >>> useful in precheckin. I think I could live with an additional 17 seconds >>> most of the time for running the spotlessCheck as suggested. >>>> >>>> Jared, Is that 17 seconds running locally on your laptop or on a more >>> capable machine? >>>> >>>> Ken >>>> >>>>> On Oct 12, 2016, at 3:39 PM, Jared Stewart <jstew...@pivotal.io> >> wrote: >>>>> >>>>> 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 <dschnei...@pivotal.io >>> >>> 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 < >> ukohlme...@pivotal.io >>>> >>>>>> 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 <dsm...@pivotal.io> >>> 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 < >>> jstew...@pivotal.io> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> +1 to running during the precheckin target as well as Travis CI >>>>>>>>>> >>>>>>>>>> On Oct 12, 2016 11:20 AM, "Darrel Schneider" < >>> dschnei...@pivotal.io> >>>>>>>>>> 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 < >>> jstew...@pivotal.io> >>>>>>>>>>> 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 < >>>>>>>>>>>>> >>>>>>>>>>>> bschucha...@pivotal.io >>>>>>>>>> >>>>>>>>>>> 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 <dsm...@pivotal.io> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> 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 < >>>>>>>>>>>>>>> >>>>>>>>>>>>>> jstew...@pivotal.io >>>>>>>>>> >>>>>>>>>>> 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. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>> >>>>> >>>> >>> >>> >> >> >> -- >> >> ~/William >>
