You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ratis.apache.org by GitBox <gi...@apache.org> on 2020/08/09 03:45:30 UTC
[GitHub] [incubator-ratis] amaliujia opened a new pull request #173: RATIS-1028. [Checkstyle] Enable AvoidImportStar
amaliujia opened a new pull request #173:
URL: https://github.com/apache/incubator-ratis/pull/173
## What changes were proposed in this pull request?
enable AvoidImportStar in checksytle config and fix existing `import *`.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-1028
## How was this patch tested?
UT
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [incubator-ratis] amaliujia commented on pull request #173: RATIS-1028. [Checkstyle] Enable AvoidStarImport
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #173:
URL: https://github.com/apache/incubator-ratis/pull/173#issuecomment-672263476
@szetszwo
I am wondering if you can commit this. If we have agreed to enable `avoid import start`, the earlier we enable it, the better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [incubator-ratis] runzhiwang commented on pull request #173: RATIS-1028. [Checkstyle] Enable AvoidStarImport
Posted by GitBox <gi...@apache.org>.
runzhiwang commented on pull request #173:
URL: https://github.com/apache/incubator-ratis/pull/173#issuecomment-671002744
+1
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [incubator-ratis] szetszwo commented on a change in pull request #173: RATIS-1028. [Checkstyle] Enable AvoidStarImport
Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #173:
URL: https://github.com/apache/incubator-ratis/pull/173#discussion_r468870549
##########
File path: ratis-common/src/main/java/org/apache/ratis/protocol/RaftGroup.java
##########
@@ -35,7 +40,8 @@ public static RaftGroup emptyGroup() {
/** @return a group with the given id and peers. */
public static RaftGroup valueOf(RaftGroupId groupId, RaftPeer... peers) {
- return new RaftGroup(groupId, peers == null || peers.length == 0? Collections.emptyList(): Arrays.asList(peers));
+ return new RaftGroup(groupId, peers == null || peers.length == 0? Collections.emptyList(): Arrays
+ .asList(peers));
Review comment:
Please revert this whitespace change. We allow 120 characters.
##########
File path: ratis-grpc/src/main/java/org/apache/ratis/grpc/client/GrpcClientStreamer.java
##########
@@ -17,23 +17,38 @@
*/
package org.apache.ratis.grpc.client;
+import java.util.Collection;
+import java.util.Deque;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
Review comment:
Why move java.util away from other java imports?
##########
File path: ratis-common/src/main/java/org/apache/ratis/util/FileUtils.java
##########
@@ -17,6 +17,12 @@
*/
package org.apache.ratis.util;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.StandardOpenOption;
Review comment:
Why move java.util away from other java imports?
##########
File path: ratis-netty/src/main/java/org/apache/ratis/netty/client/NettyClientRpc.java
##########
@@ -30,6 +29,14 @@
import org.apache.ratis.proto.netty.NettyProtos.RaftNettyServerRequestProto;
import java.io.IOException;
+import org.apache.ratis.protocol.ClientId;
+import org.apache.ratis.protocol.GroupInfoRequest;
+import org.apache.ratis.protocol.GroupListRequest;
+import org.apache.ratis.protocol.GroupManagementRequest;
+import org.apache.ratis.protocol.RaftClientReply;
+import org.apache.ratis.protocol.RaftClientRequest;
+import org.apache.ratis.protocol.RaftPeerId;
+import org.apache.ratis.protocol.SetConfigurationRequest;
Review comment:
This should move to next to the other org.apache.ratis imports.
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
##########
@@ -515,7 +571,7 @@ NotLeaderException generateNotLeaderException() {
return new NotLeaderException(getMemberId(), conf.getPeer(leaderId), peers);
}
- private LifeCycle.State assertLifeCycleState(Set<LifeCycle.State> expected) throws ServerNotReadyException {
+ private LifeCycle.State assertLifeCycleState(Set<State> expected) throws ServerNotReadyException {
Review comment:
Let's keep using LifeCycle.State.
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
##########
@@ -17,9 +17,57 @@
*/
package org.apache.ratis.server.impl;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.OptionalLong;
+import java.util.Set;
Review comment:
This should move to next to the other java imports.
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLogCache.java
##########
@@ -17,6 +17,13 @@
*/
package org.apache.ratis.server.raftlog.segmented;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.Objects;
Review comment:
This should move to next to the other java imports.
##########
File path: ratis-logservice/src/main/java/org/apache/ratis/logservice/api/LogServiceClient.java
##########
@@ -25,16 +25,23 @@
import org.apache.ratis.logservice.impl.ExportedLogStreamImpl;
import org.apache.ratis.logservice.impl.LogStreamImpl;
import org.apache.ratis.logservice.proto.LogServiceProtos;
-import org.apache.ratis.logservice.proto.MetaServiceProtos.*;
+import org.apache.ratis.logservice.proto.MetaServiceProtos.CreateLogReplyProto;
+import org.apache.ratis.logservice.proto.MetaServiceProtos.DeleteLogReplyProto;
+import org.apache.ratis.logservice.proto.MetaServiceProtos.GetLogReplyProto;
+import org.apache.ratis.logservice.proto.MetaServiceProtos.ListLogsReplyProto;
+import org.apache.ratis.logservice.proto.MetaServiceProtos.LogInfoProto;
import org.apache.ratis.logservice.server.ArchivalInfo;
import org.apache.ratis.logservice.util.LogServiceProtoUtil;
import org.apache.ratis.logservice.util.MetaServiceProtoUtil;
-import org.apache.ratis.protocol.*;
import java.io.IOException;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
+import org.apache.ratis.protocol.ClientId;
+import org.apache.ratis.protocol.RaftClientReply;
+import org.apache.ratis.protocol.RaftGroup;
+import org.apache.ratis.protocol.RaftPeer;
Review comment:
This should move to next to the other org.apache.ratis imports.
##########
File path: ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java
##########
@@ -17,6 +17,12 @@
*/
package org.apache.ratis.grpc.server;
+import java.util.LinkedList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Queue;
+import java.util.UUID;
Review comment:
Why move java.util away from other java imports?
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/LogAppender.java
##########
@@ -17,7 +17,20 @@
*/
package org.apache.ratis.server.impl;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.UUID;
Review comment:
This should move to next to the other java imports.
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderState.java
##########
@@ -17,9 +17,27 @@
*/
package org.apache.ratis.server.impl;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
Review comment:
This should move to next to the other java imports.
##########
File path: ratis-examples/src/main/java/org/apache/ratis/examples/filestore/FileStoreCommon.java
##########
@@ -18,11 +18,14 @@
package org.apache.ratis.examples.filestore;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
-import org.apache.ratis.util.*;
import java.io.IOException;
import java.nio.file.Path;
import java.util.concurrent.CompletableFuture;
+import org.apache.ratis.util.JavaUtils;
+import org.apache.ratis.util.ProtoUtils;
+import org.apache.ratis.util.SizeInBytes;
+import org.apache.ratis.util.TraditionalBinaryPrefix;
Review comment:
Why move org.apache.ratis.util away from other org.apache.ratis imports?
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java
##########
@@ -18,15 +18,20 @@
package org.apache.ratis.server.impl;
import org.apache.ratis.conf.RaftProperties;
-import org.apache.ratis.protocol.*;
+import org.apache.ratis.protocol.RaftGroup;
+import org.apache.ratis.protocol.RaftGroupId;
+import org.apache.ratis.protocol.RaftGroupMemberId;
+import org.apache.ratis.protocol.RaftPeerId;
+import org.apache.ratis.protocol.StateMachineException;
import org.apache.ratis.server.RaftServerConfigKeys;
import org.apache.ratis.server.protocol.TermIndex;
import org.apache.ratis.server.raftlog.RaftLog;
import org.apache.ratis.server.raftlog.memory.MemoryRaftLog;
import org.apache.ratis.server.raftlog.segmented.SegmentedRaftLog;
-import org.apache.ratis.server.storage.*;
import org.apache.ratis.proto.RaftProtos.InstallSnapshotRequestProto;
import org.apache.ratis.proto.RaftProtos.LogEntryProto;
+import org.apache.ratis.server.storage.RaftStorage;
+import org.apache.ratis.server.storage.SnapshotManager;
Review comment:
Let's put the org.apache.ratis.server together.
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLogReader.java
##########
@@ -17,6 +17,16 @@
*/
package org.apache.ratis.server.raftlog.segmented;
+import java.io.BufferedInputStream;
+import java.io.Closeable;
+import java.io.DataInputStream;
+import java.io.EOFException;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.FilterInputStream;
+import java.io.IOException;
+import java.io.InputStream;
Review comment:
This should move to next to the other java imports.
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerProxy.java
##########
@@ -17,6 +17,13 @@
*/
package org.apache.ratis.server.impl;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionException;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
Review comment:
This should move to next to the other java imports.
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/StateMachineUpdater.java
##########
@@ -52,7 +56,7 @@
* asynchronously so that this will not block normal raft protocol.
*
* If the auto log compaction is enabled, the state machine updater thread will
- * trigger a snapshot of the state machine by calling
+ * trigger a snapshot of the state machine by callingestRestartFollower/
Review comment:
Why changing this?
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/RaftConfiguration.java
##########
@@ -17,11 +17,15 @@
*/
package org.apache.ratis.server.impl;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
Review comment:
This should move to next to the other java imports.
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/storage/RaftStorageDirectory.java
##########
@@ -17,6 +17,12 @@
*/
package org.apache.ratis.server.storage;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Objects;
+import java.util.UUID;
Review comment:
This should move to next to the other java imports.
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/storage/MetaFile.java
##########
@@ -17,12 +17,18 @@
*/
package org.apache.ratis.server.storage;
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
Review comment:
This should move to next to the other java imports.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [incubator-ratis] amaliujia commented on pull request #173: RATIS-1028. [Checkstyle] Enable AvoidStarImport
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #173:
URL: https://github.com/apache/incubator-ratis/pull/173#issuecomment-672324312
Thank you!
Those changes are automatically done by my IntelliJ which has configured by CheckStyle, and I assumed checks pass means these changes are good to Ratis
I will take another look on your comments about java import ordering. But I also agreed we should put more time on other areas. So if such checkstyle work needs too much time, I will postpone this change.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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