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. > > > > > > > > > > > > >
