geode-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (GEODE-3952) Improve spotless to more closely adhere to established style guidelines.
Date Fri, 03 Nov 2017 18:11:00 GMT

    [ https://issues.apache.org/jira/browse/GEODE-3952?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16238110#comment-16238110
] 

ASF GitHub Bot commented on GEODE-3952:
---------------------------------------

PurelyApplied commented on a change in pull request #1015: GEODE-3952: Improve Spotless
URL: https://github.com/apache/geode/pull/1015#discussion_r148857088
 
 

 ##########
 File path: build.gradle
 ##########
 @@ -97,16 +97,58 @@ subprojects {
 
   apply plugin: "com.diffplug.gradle.spotless"
   spotless {
-    lineEndings = 'unix';
+    lineEndings = 'unix'
     java {
       target project.fileTree(project.projectDir) {
         include '**/*.java'
         exclude '**/generated-src/**'
       }
+
+      custom 'Remove commented-out import statements', {
+        it.replaceAll(/\n\/\/ import .*?;.*/, '')
+      }
+
+      custom 'End files with a single blank line', {
+        it.replaceAll(/\s+$/, '\n')
+      }
+
+      // Removes end-of-line whitespace
+      trimTrailingWhitespace()
+
+      // Enforce style import order
+      importOrderFile "${project(':geode-core').projectDir}/../etc/eclipseOrganizeImports.importorder"
+
+      // Enforce style modifier order
+      custom 'Modifier ordering', {
+        def modifierRanking = [
+                public      : 1,
+                protected   : 2,
+                private     : 3,
+                abstract    : 4,
+                default     : 5,
+                static      : 6,
+                final       : 7,
+                transient   : 8,
+                volatile    : 9,
+                synchronized: 10,
+                native      : 11,
+                strictf     : 12]
+        // Find any instance of multiple modifiers. Lead with a non-word character to avoid
+        // accidental matching against for instance, "an alternative default value"
+        it.replaceAll(/\W(?:public |protected |private |abstract |default |static |final
|transient |volatile |synchronized |native |strictfp ){2,}/, {
 
 Review comment:
   Line 318 is attempting to match any substring that (1) starts with a non-word character
and (2) contains two or more modifiers.  That substring then has the match modifiers reordered.
   
   The non-word character is required to avoid finding `native default` in the string "alternative
default value` and "reordering" it to "alterdefault native value".
   
   This will match comments, and indeed the "an alternative default value" was found in a
docstring in `BaseValueHolder` test.  At time of development, the only comments matched were
commented-out code with incorrect modifiers.
   
   I suppose it could be extended to also include optional whitespace after a carriage return
to avoid picking up comments or javadocs.  Would this be preferable?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


> Improve spotless to more closely adhere to established style guidelines. 
> -------------------------------------------------------------------------
>
>                 Key: GEODE-3952
>                 URL: https://issues.apache.org/jira/browse/GEODE-3952
>             Project: Geode
>          Issue Type: Improvement
>            Reporter: Patrick Rhomberg
>
> Spotless currently addresses whitespacing and very little else.  It could be expanded
to clean up the more glaring (in my opinion) broken windows.  This includes:
> * Adherence to the established import ordering given in `[geode]/etc/`
> * Adherence to the established modifier ordering given in the Google Java Style guide,
which we largely adopt.  (e.g., `public static int` over `static public int`.)
> * Removal of some dead code, particularly commented-out import statements.
> * Improvements to whitespace handling, particularly to end files in a single carriage
return and to trim whitespace from the end of each line.
> Increase to compile time is trivial, approximately ten seconds in the worst case.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Mime
View raw message