-
Notifications
You must be signed in to change notification settings - Fork 13k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Method name optimization #3104
Method name optimization #3104
Conversation
@KomachiSion Hello, I have optimized the name of the hump in the method name of the main logic, could you please have a look |
@KomachiSion Hello, I may have some spaces when Reformat the code |
@pengzhengfa Please use This file is under dir of source code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have unified code style for naming-module.
Please use unified code style to reformat. Thanks.
Otherwise we will not merge PR.
ok |
Please see If you use eclipse, Please only change these code you want to changed. And we are welcome to contribute eclipse code style file according to |
@KomachiSion Hello, I have solved it. Please check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one code style problem need to change
and one naming suggestion.
@@ -312,7 +312,7 @@ public void updateIps(List<Instance> ips, boolean ephemeral) { | |||
|
|||
Map<String, Integer> intersectMap = new ConcurrentHashMap<>(newInstance.size() + oldInstance.size()); | |||
Map<String, Instance> instanceMap = new ConcurrentHashMap<>(newInstance.size()); | |||
Map<String, Instance> instanceMap1 = new ConcurrentHashMap<>(newInstance.size()); | |||
Map<String, Instance> instancesMap = new ConcurrentHashMap<>(newInstance.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read method again and I think we should change instanceMap
to updatedInstancesMap
change instanceMap1
to newInstancesMap
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I change
@@ -329,8 +329,8 @@ public void updateIps(List<Instance> ips, boolean ephemeral) { | |||
intersectMap.put(instance.toString(), 1); | |||
} | |||
} | |||
|
|||
instanceMap1.put(instance.toString(), instance); | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not change the Indentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I change
Please do not create a Pull Request without creating an issue first.
What is the purpose of the change
XXXXX
Brief changelog
XX
Verifying this change
XXXX
Follow this checklist to help us incorporate your contribution quickly and easily:
[ISSUE #123] Fix UnknownException when host config not exist
. Each commit in the pull request should have a meaningful subject line and body.mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=true
to make sure basic checks pass. Runmvn clean install -DskipITs
to make sure unit-test pass. Runmvn clean test-compile failsafe:integration-test
to make sure integration-test pass.