I totally understand your frustrations, Kirk. It is disheartening to stub your toe on someone else's rough corners.
This email became more verbose than I intended, which is par for the course for me, but still... tl;dr: you can change whitespace rules in the etc/eclipse*.xml. Feel free to go relax those rules, if you can figure out which exactly which constants Eclipse wants set to what and where. Personally, I don't have a horse in this race on what the whitespace rules should be. I just think that we should have a standard that is enforced. ----- Just to clarify some possible misconceptions: * We don't actually use google-java-format utility. Spotless is provided by DiffPlug and accepts an arbitrary Eclipse format file. * We _do_ use a format spec that was initially based off the Google Java format style guide. It can be found at the now-suboptimally-named <geode>/etc/eclipse-java-google-style.xml. * (Of course, problems in the google-java-format utility may share a root cause of in that Eclipse format xml, so that google-java-format issue may indeed be what we were hitting.) * The Eclipse format xml should contain just about everything that deals with whitespacing in our style. * Everything else that spotless cares about is contained in <geode>/gradle/spotless.gradle * Most of our not-whitespace-related spotless constraints came from observing repeated violations (read: "petty annoyances") throughout our own codebase. Still, with few exceptions, you really shouldn't be hitting any non-whitespace spotless issues if you are adhering to standard practices. Our current spotless rules are as follows: ==== Best practices rules: * Remove your unused imports. * Remove your commented-out imports * Do not use wildcard imports * Remove empty Javadoc stubs (e.g., "@param myParam" with no description). Better would be to write descriptive javadocs. * Empty javadoc or block comments. ==== Whitespace rules: * The litany of things defined in the Eclipse format file. (For instance, if there is a setting for spaces after punctuation in comments, it would live here somewhere.) * Lines should not end with whitespace before the newline. * A file should end with a newline. ==== Community standards and for consistency rules: * We have an opinion on import order (Historically, there was a reorder-war between Eclipse and IntelliJ users that disagreed on where javax. should go. This was standardized to reduce diff noise in pull requests.) * We have an opinion on modifier word order. (This appears to be more of a Java-community standard. Not a "best practice" per se, but helps with readability.) ==== Known issue: * There is some disagreement between the IntelliJ's style file in etc/ and the Eclipse style file in the same directory. * As a corollary and because Spotless uses the Eclipse style file, running AutoFormat in IntelliJ can create formatting that is considered a violation of spotless. And that, everyone, is pretty much everything that I know about Spotless. Imagination is Change. ~Patrick On Wed, Aug 22, 2018 at 9:55 AM, Anthony Baker <aba...@pivotal.io> wrote: > Thanks Kirk! FWIW, I’m also annoyed with the overzealous spotless > constraints. > > Anthony > > > > On Aug 22, 2018, at 9:28 AM, Kirk Lund <kl...@apache.org> wrote: > > > > Sorry to waste everyone’s time with something so trivial. I tried hard to > > ensure my instructions for Setting Up IntelliJ were correct and > error-free, > > so this formatter issue was a big disappointment to me because of that. > The > > instructions have been updated. > > > > On Tue, Aug 21, 2018 at 3:57 PM, Kirk Lund <kl...@apache.org> wrote: > > > >> Looks like it's a feature: https://github.com/google/ > google-java-format/ > >> issues/62 > >> > >> Is it too late to down-vote our use of google-java-format? > >> > >> On Tue, Aug 21, 2018 at 3:43 PM, Kirk Lund <kl...@apache.org> wrote: > >> > >>> I suppose I was using that older format of the Apache license header > and > >>> then using spotlessApply 100% before running spotlessCheck which was > >>> reformatting the license header. So even though I was using the older > one, > >>> I never ran into the problem until today. > >>> > >>> So maybe nothing changed? > >>> > >>> But, I still think it's ridiculous that we have spotless configured to > >>> disallow a double-space after sentence terminator. > >>> > >>> On Tue, Aug 21, 2018 at 3:39 PM, Kirk Lund <kl...@apache.org> wrote: > >>> > >>>> I know it's not a bug in spotless. I think we now have the settings a > >>>> bit too strict. > >>>> > >>>> As of 2-3 weeks ago, I was able to follow the "Setting up IntelliJ" > >>>> process that I documented at https://github.com/gemfire/gemfire > (search > >>>> down for "Setting up IntelliJ") without spotless failing. See the > >>>> format of the Apache license header that's pasted into that readme? > It has > >>>> the extra spaces, including 2 spaces between sentences. > >>>> > >>>> 2-3 weeks ago, this was working fine. Now it fails spotless, so > >>>> something changed. Maybe the version of spotless that we're using in > >>>> gradle? Or a gradle spotless plugin version changed? > >>>> > >>>> At best, it's laughable that our spotless format now complains about > >>>> correct English syntax in comments and javadocs. At worst, it's > evidence > >>>> that our use of spotless is... "a bit too strict" which in my opinion > >>>> should be fixed. > >>>> > >>>> Can you please look into what changed? I haven't had much luck finding > >>>> it yet but I assure you that something did change. > >>>> > >>>> On Tue, Aug 21, 2018 at 2:26 PM, Patrick Rhomberg < > prhomb...@pivotal.io> > >>>> wrote: > >>>> > >>>>> The only addition with respect to spotless on the 10th was to add the > >>>>> `devBuild` target (which runs `spotlessApply`) and to require that > >>>>> `spotlessApply` would run before `compileJava`, if both were to run > in a > >>>>> given build command. > >>>>> > >>>>> Looking at the PR against which these failed, it looks like it might > be > >>>>> some disagreement between your IDE's desired format and spotless's. > >>>>> Notably, the new test file header is thinner and has more space > >>>>> padding. I > >>>>> hadn't thought spotless cared about comment blocks, but looking now, > it > >>>>> does look like we're consistent everywhere else (within the Java code > >>>>> that > >>>>> spotless targets) on how that header is formatted. > >>>>> > >>>>> So, you know... It's a feature, not a bug? And we should investigate > >>>>> the > >>>>> discrepancies between the format files in <geode>/etc, that is, the > >>>>> Eclipse > >>>>> file spotless uses and the IntelliJ file that is meant to emulate it. > >>>>> > >>>>> On Tue, Aug 21, 2018 at 9:48 AM, Kirk Lund <kl...@apache.org> wrote: > >>>>> > >>>>>> This appears to be caused by changes made to the build around August > >>>>> 10? > >>>>>> > >>>>>> On Tue, Aug 21, 2018 at 9:38 AM, Kirk Lund <kl...@apache.org> > wrote: > >>>>>> > >>>>>>> Why is spotless now complaining about correct English? By correct > >>>>>> English, > >>>>>>> I mean having 2 spaces between sentences in javadoc or comments (in > >>>>> this > >>>>>>> case it's the Apache license header): > >>>>>>> > >>>>>>> -·*·the·License.··You·may·obtain·a·copy·of·the·License·at > >>>>>>> +·*·the·License.·You·may·obtain·a·copy·of·the·License·at > >>>>>>> > >>>>>>> Execution failed for task ':geode-core:spotlessJava'. > >>>>>>> <https://concourse.apachegeode-ci.info/builds/19648#L5b60a2a0:1903 > >>>>>> > >>>>>>>> The following files had format violations: > >>>>>>> <https://concourse.apachegeode-ci.info/builds/19648#L5b60a2a0:1904 > >>>>>> > >>>>>>> geode-core/src/main/java/org/apache/geode/internal/cache/ > >>>>>> RegionNameValidation.java > >>>>>>> <https://concourse.apachegeode-ci.info/builds/19648#L5b60a2a0:1905 > >>>>>> > >>>>>>> @@ -1,12 +1,12 @@ > >>>>>>> <https://concourse.apachegeode-ci.info/builds/19648#L5b60a2a0:1906 > >>>>>> > >>>>>>> /* > >>>>>>> <https://concourse.apachegeode-ci.info/builds/19648#L5b60a2a0:1907 > >>>>>> > >>>>>>> ·*·Licensed·to·the·Apache·Software·Foundation·(ASF)· > >>>>>> under·one·or·more > >>>>>>> <https://concourse.apachegeode-ci.info/builds/19648#L5b60a2a0:1908 > >>>>>> > >>>>>>> -·*·contributor·license·agreements.··See·the·NOTICE· > >>>>>> file·distributed·with > >>>>>>> <https://concourse.apachegeode-ci.info/builds/19648#L5b60a2a0:1909 > >>>>>> > >>>>>>> +·*·contributor·license·agreements.·See·the·NOTICE· > >>>>>> file·distributed·with > >>>>>>> <https://concourse.apachegeode-ci.info/builds/19648#L5b60a2a0:1910 > >>>>>> > >>>>>>> ·*·this·work·for·additional·information·regarding· > >>>>>> copyright·ownership. > >>>>>>> <https://concourse.apachegeode-ci.info/builds/19648#L5b60a2a0:1911 > >>>>>> > >>>>>>> ·*·The·ASF·licenses·this·file·to·You·under·the·Apache· > >>>>>> License,·Version·2.0 > >>>>>>> <https://concourse.apachegeode-ci.info/builds/19648#L5b60a2a0:1912 > >>>>>> > >>>>>>> ·*·(the·"License");·you·may·not·use·this·file·except·in· > >>>>>> compliance·with > >>>>>>> <https://concourse.apachegeode-ci.info/builds/19648#L5b60a2a0:1913 > >>>>>> > >>>>>>> -·*·the·License.··You·may·obtain·a·copy·of·the·License·at > >>>>>>> <https://concourse.apachegeode-ci.info/builds/19648#L5b60a2a0:1914 > >>>>>> > >>>>>>> +·*·the·License.·You·may·obtain·a·copy·of·the·License·at > >>>>>>> <https://concourse.apachegeode-ci.info/builds/19648#L5b60a2a0:1915 > >>>>>> > >>>>>>> ·* > >>>>>>> <https://concourse.apachegeode-ci.info/builds/19648#L5b60a2a0:1916 > >>>>>> > >>>>>>> -·*······http://www.apache.org/licenses/LICENSE-2.0 > >>>>>>> <https://concourse.apachegeode-ci.info/builds/19648#L5b60a2a0:1917 > >>>>>> > >>>>>>> +·*·http://www.apache.org/licenses/LICENSE-2.0 > >>>>>>> <https://concourse.apachegeode-ci.info/builds/19648#L5b60a2a0:1918 > >>>>>> > >>>>>>> ·* > >>>>>>> <https://concourse.apachegeode-ci.info/builds/19648#L5b60a2a0:1919 > >>>>>> > >>>>>>> ·*·Unless·required·by·applicable·law·or·agreed·to· > >>>>>> in·writing,·software > >>>>>>> <https://concourse.apachegeode-ci.info/builds/19648#L5b60a2a0:1920 > >>>>>> > >>>>>>> ·*·distributed·under·the·License·is·distributed·on·an·" > >>>>>> AS·IS"·BASIS, > >>>>>>> > >>>>>>> > >>>>>>> > >>>>>> > >>>>> > >>>> > >>>> > >>> > >> > >
