fix hugegraph-api code checkstyle by chuanxo · Pull Request #1845 · apache/hugegraph

chuanxo · 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

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

                                         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

    }

-    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

    }

-    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

chuanxo · 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

chuanxo 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

chuanxo mentioned this pull request Apr 25, 2022

fix hugegraph-api code checkstyle #1851

Merged

chuanxo closed this Apr 26, 2022

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

Conversation

chuanxo commented Apr 24, 2022

Uh oh!

codecov bot commented Apr 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

imbajin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imbajin Apr 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imbajin commented Apr 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chuanxo commented Apr 25, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

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