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