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

Reply via email to