-
Notifications
You must be signed in to change notification settings - Fork 93
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
refact: fix some code style with check plugin #385
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #385 +/- ##
============================================
- Coverage 62.58% 62.54% -0.04%
- Complexity 894 1866 +972
============================================
Files 91 260 +169
Lines 4383 9407 +5024
Branches 515 872 +357
============================================
+ Hits 2743 5884 +3141
- Misses 1437 3141 +1704
- Partials 203 382 +179 ☔ View full report in Codecov by Sentry. |
@@ -54,6 +58,7 @@ public static ServerException fromResponse(Response response) { | |||
exception.cause = (String) getByKeys(json, CAUSE_KEYS); | |||
exception.trace = getByKeys(json, TRACE_KEYS); | |||
} catch (Exception ignored) { | |||
LOG.error(" ServerException excepiton "); |
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.
we need log here? @javeme
and seems have 2 extra spaces
@@ -57,6 +61,7 @@ public HugeConfig hugeConfig() { | |||
conf = path; | |||
} | |||
} catch (Exception ignored) { | |||
LOG.error("hugeConfig exception"); |
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.
same question
vMapping = new org.apache.hugegraph.loader.mapping.VertexMapping( | ||
idFields.get(0), true); |
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.
if line < 100, use one line instead
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.
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/flink/HugeGraphOutputFormat.java
Outdated
Show resolved
Hide resolved
request.vertices((List<Vertex>) (Object) graphElements) | ||
.updatingStrategies(updateStrategyMap) | ||
.createIfNotExist(true); |
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.
align problem?
.updatingStrategies(updateStrategyMap) | ||
.checkVertex(isCheckVertex) | ||
.createIfNotExist(true); |
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.
same, and why we change the name? (seems optional)
.updatingStrategies(this.mapping.updateStrategies()) | ||
.createIfNotExist(true); |
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.
same, and so as others
|
||
public class ServerException extends RuntimeException { | ||
|
||
private static final long serialVersionUID = 6335623004322652358L; | ||
|
||
protected static final Logger LOG = Log.logger(ServerException.class); |
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.
mark as private?
@@ -33,6 +35,8 @@ | |||
@Configuration | |||
public class HubbleConfig { | |||
|
|||
protected static final Logger LOG = Log.logger(HubbleConfig.class); |
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.
ditto
g.updateVertices(req.build()); | ||
BatchVertexRequest.Builder request = new BatchVertexRequest.Builder(); | ||
request.vertices((List<Vertex>) (Object) graphElements) | ||
.updatingStrategies(updateStrategyMap) |
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.
align with '.'
.createIfNotExist(true); | ||
BatchEdgeRequest.Builder request = new BatchEdgeRequest.Builder(); | ||
request.edges((List<Edge>) (Object) elements) | ||
.updatingStrategies(this.mapping.updateStrategies()) |
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.
ditto
"not allowed to insert, because already exist a vertex " + | ||
"Connection refused", | ||
"The server is being shutting down", | ||
"not allowed to insert, because already exist a vertex " + |
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.
keep the origin style?
@seagle-yuan will this PR fixed before release? (before next week) or just leave it in next release |
Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label |
add(peter); | ||
add(lop); | ||
add(vadasB); | ||
}}; |
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.
can we keep the origin style?
add(peterCreateLop); | ||
}}; | ||
add(peterCreateLop); | ||
}}; |
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.
can we keep the origin style?
Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label |
Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label |
Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label |
Hi, @seagle-yuan, any update with this? |
could follow this PR and finish it |
Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label |
Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #385 +/- ##
============================================
- Coverage 62.58% 62.54% -0.04%
- Complexity 894 1866 +972
============================================
Files 91 260 +169
Lines 4383 9407 +5024
Branches 515 872 +357
============================================
+ Hits 2743 5884 +3141
- Misses 1437 3141 +1704
- Partials 203 382 +179 ☔ View full report in Codecov by Sentry. |
Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label |
Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label |
Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label |
@imbajin could you move this PR forward? |
@liuxiaocs7 is working on it but busy for now (hold it on) Also this PR is not urgent(low priority) |
Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label |
Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label |
fix checkstyle check