fix hugegraph-api code checkstyle #1845

Fizell · 2022-04-24T11:07:59Z

rt

codecov · 2022-04-24T11:17:08Z

Codecov Report

Merging #1845 (f28f9bd) into master (9a8259e) will decrease coverage by 0.01%.
The diff coverage is 76.92%.

@@             Coverage Diff              @@
##             master    #1845      +/-   ##
============================================
- Coverage     66.93%   66.92%   -0.02%     
- Complexity      444      731     +287     
============================================
  Files           446      446              
  Lines         37966    37948      -18     
  Branches       5410     5410              
============================================
- Hits          25413    25397      -16     
+ Misses         9957     9956       -1     
+ Partials       2596     2595       -1

Impacted Files	Coverage Δ
.../java/com/baidu/hugegraph/api/auth/ProjectAPI.java	`82.05% <ø> (ø)`
...va/com/baidu/hugegraph/api/profile/ProfileAPI.java	`0.00% <0.00%> (ø)`
...aidu/hugegraph/auth/WsAndHttpBasicAuthHandler.java	`58.62% <0.00%> (+1.47%)`	⬆️
...a/com/baidu/hugegraph/license/LicenseVerifier.java	`0.00% <0.00%> (ø)`
...om/baidu/hugegraph/auth/StandardAuthenticator.java	`44.73% <50.00%> (+0.73%)`	⬆️
.../com/baidu/hugegraph/server/ApplicationConfig.java	`93.02% <50.00%> (-0.46%)`	⬇️
...com/baidu/hugegraph/auth/HugeFactoryAuthProxy.java	`57.92% <62.50%> (+0.20%)`	⬆️
...va/com/baidu/hugegraph/auth/HugeAuthenticator.java	`40.10% <66.66%> (-0.11%)`	⬇️
.../java/com/baidu/hugegraph/metrics/MetricsUtil.java	`62.50% <66.66%> (ø)`
...api/src/main/java/com/baidu/hugegraph/api/API.java	`74.11% <70.00%> (-0.89%)`	⬇️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9a8259e...f28f9bd. Read the comment docs.

imbajin

Thanks for your contribution~ 👍🏻

seems a lot of the code style conflict, maybe we could use a unified rule before change it

imbajin · 2022-04-24T13:07:31Z

hugegraph-api/src/main/java/com/baidu/hugegraph/auth/WsAndHttpBasicAuthHandler.java

@@ -155,7 +151,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
        private void sendError(ChannelHandlerContext ctx, Object msg) {
            // Close the connection as soon as the error message is sent.
            ctx.writeAndFlush(new DefaultFullHttpResponse(HTTP_1_1, UNAUTHORIZED))
-               .addListener(ChannelFutureListener.CLOSE);
+                .addListener(ChannelFutureListener.CLOSE);


maybe we should keep the origin align?

but the current style check specify that the code indented in multiples of 4，so we need to decide which style to use

but the current style check specify that the code indented in multiples of 4，so we need to decide which style to use

yep, we'll discuss this issue in the developer group, ensure the rule

imbajin · 2022-04-24T13:08:27Z

hugegraph-api/src/main/java/com/baidu/hugegraph/auth/WsAndHttpBasicAuthHandler.java

-                                                  Charset.forName("UTF-8"));
+                        StandardCharsets.UTF_8);


we use the hugegraph-style in the wiki (not java original style)

align the param with the highly priority

imbajin · 2022-04-24T13:10:40Z

hugegraph-api/src/main/java/com/baidu/hugegraph/auth/WsAndHttpBasicAuthHandler.java

+import static io.netty.handler.codec.http.HttpHeaderNames.CONNECTION;
+import static io.netty.handler.codec.http.HttpHeaderNames.UPGRADE;
+import static io.netty.handler.codec.http.HttpResponseStatus.UNAUTHORIZED;
+import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
+import static org.apache.tinkerpop.gremlin.groovy.jsr223.dsl.credential.CredentialGraphTokens.PROPERTY_PASSWORD;
+import static org.apache.tinkerpop.gremlin.groovy.jsr223.dsl.credential.CredentialGraphTokens.PROPERTY_USERNAME;


so as these import rules, u could refer the style file in root dir to improve them

imbajin · 2022-04-24T13:14:14Z

hugegraph-api/src/main/java/com/baidu/hugegraph/auth/StandardAuthenticator.java

-            System.out.println(notEmptyPrompt);
+            LOG.info(notEmptyPrompt);


seems it don't print in the log file (print on the console is right?) @javeme

yes we need System.out sometimes like when using the console

imbajin · 2022-04-24T13:14:32Z

hugegraph-api/src/main/java/com/baidu/hugegraph/auth/HugeGraphAuthProxy.java

@@ -1757,7 +1731,7 @@ public static class ContextThreadPoolExecutor extends ThreadPoolExecutor {
        public ContextThreadPoolExecutor(int corePoolSize, int maxPoolSize,
                                         ThreadFactory threadFactory) {
            super(corePoolSize, maxPoolSize, 0L, TimeUnit.MILLISECONDS,
-                  new LinkedBlockingQueue<Runnable>(), threadFactory);
+                    new LinkedBlockingQueue<>(), threadFactory);


keep the origin align~

same as above

imbajin · 2022-04-25T04:28:03Z

Open a new vote in #1848. we could discuss there.

javeme · 2022-04-25T12:58:45Z

hugegraph-api/src/main/java/com/baidu/hugegraph/api/graph/BatchAPI.java


    static {
        MetricsUtil.registerGauge(RestServer.class, "batch-write-threads",
-                                  () -> batchWriteThreads.intValue());
+                BATCH_WRITE_THREADS::intValue);


align with RestServer

javeme · 2022-04-25T13:01:11Z

hugegraph-api/src/main/java/com/baidu/hugegraph/auth/HugeAuthenticator.java

@@ -254,7 +248,7 @@ public static class UserJson {
        }
    }

-    public static class RolePerm {
+    class RolePerm {


prefer to keep public static class

javeme · 2022-04-25T13:01:20Z

hugegraph-api/src/main/java/com/baidu/hugegraph/auth/HugeAuthenticator.java

@@ -394,7 +388,7 @@ public static boolean match(Object role, RolePermission grant,
        }
    }

-    public static class RequiredPerm {
+    class RequiredPerm {


prefer to keep public static class

javeme · 2022-04-25T13:01:52Z

hugegraph-api/src/main/java/com/baidu/hugegraph/auth/HugeFactoryAuthProxy.java


    private static final Set<String> PROTECT_METHODS = ImmutableSet.of(
-                                                       "instance");
+            "instance");


align with ImmutableSet

javeme · 2022-04-25T13:04:19Z

hugegraph-api/src/main/java/com/baidu/hugegraph/auth/HugeGraphAuthProxy.java

-        return verifySchemaPermission(HugePermission.READ, () -> {
-            return this.hugegraph.propertyKey(key);
-        });
+        return verifySchemaPermission(HugePermission.READ, () -> this.hugegraph.propertyKey(key));


prefer to keep origin style for debug and readability

javeme · 2022-04-25T13:06:54Z

hugegraph-api/src/main/java/com/baidu/hugegraph/auth/StandardAuthenticator.java

-            System.out.println(notEmptyPrompt);
+            LOG.info(notEmptyPrompt);


yes we need System.out sometimes like when using the console

Fizell · 2022-04-25T14:44:36Z

seems that tends to keep the original align and keep original lambda style, so I submit a new PR to fix only some obvious style issues #1851, and this PR will be closed @javeme @imbajin

Fizell and others added 3 commits April 19, 2022 00:51

fix hugegraph-api code checkstyle

75fb129

Merge branch 'apache:master' into fix-style

1dfc304

Merge branch 'apache:master' into fix-style

f28f9bd

imbajin reviewed Apr 24, 2022

View reviewed changes

javeme requested changes Apr 25, 2022

View reviewed changes

Fizell mentioned this pull request Apr 25, 2022

fix hugegraph-api code checkstyle #1851

Merged

Fizell closed this Apr 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix hugegraph-api code checkstyle #1845

fix hugegraph-api code checkstyle #1845

Fizell commented Apr 24, 2022

codecov bot commented Apr 24, 2022 •

edited

Loading

imbajin left a comment •

edited

Loading

imbajin Apr 24, 2022

Fizell Apr 24, 2022

imbajin Apr 25, 2022

imbajin Apr 24, 2022

imbajin Apr 24, 2022

imbajin Apr 24, 2022 •

edited

Loading

javeme Apr 25, 2022

imbajin Apr 24, 2022

Fizell Apr 24, 2022

imbajin commented Apr 25, 2022 •

edited

Loading

javeme Apr 25, 2022

javeme Apr 25, 2022

javeme Apr 25, 2022

javeme Apr 25, 2022

javeme Apr 25, 2022

javeme Apr 25, 2022

Fizell commented Apr 25, 2022

		System.out.println(notEmptyPrompt);
		LOG.info(notEmptyPrompt);

fix hugegraph-api code checkstyle #1845

fix hugegraph-api code checkstyle #1845

Conversation

Fizell commented Apr 24, 2022

codecov bot commented Apr 24, 2022 • edited Loading

Codecov Report

imbajin left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imbajin Apr 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imbajin commented Apr 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fizell commented Apr 25, 2022

codecov bot commented Apr 24, 2022 •

edited

Loading

imbajin left a comment •

edited

Loading

imbajin Apr 24, 2022 •

edited

Loading

imbajin commented Apr 25, 2022 •

edited

Loading