Skip to content
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

fix redundant Checkstyle #1896

Merged
merged 21 commits into from
Oct 30, 2022
Merged

Conversation

seagle-yuan
Copy link
Contributor

add redundant word check

@seagle-yuan seagle-yuan changed the title Checkstyle redundant fix redundant Checkstyle Jun 5, 2022
@codecov
Copy link

codecov bot commented Jun 5, 2022

Codecov Report

Merging #1896 (868ee2f) into master (9b5950e) will increase coverage by 0.04%.
The diff coverage is 82.60%.

@@             Coverage Diff              @@
##             master    #1896      +/-   ##
============================================
+ Coverage     70.54%   70.58%   +0.04%     
+ Complexity      978      724     -254     
============================================
  Files           454      454              
  Lines         39055    39054       -1     
  Branches       5557     5557              
============================================
+ Hits          27550    27567      +17     
+ Misses         8806     8791      -15     
+ Partials       2699     2696       -3     
Impacted Files Coverage Δ
...n/java/com/baidu/hugegraph/api/auth/AccessAPI.java 0.00% <ø> (ø)
...ava/com/baidu/hugegraph/api/gremlin/CypherAPI.java 0.00% <0.00%> (ø)
...va/com/baidu/hugegraph/api/gremlin/GremlinAPI.java 100.00% <ø> (ø)
...a/com/baidu/hugegraph/auth/HugeGraphAuthProxy.java 57.51% <ø> (ø)
...aidu/hugegraph/auth/WsAndHttpBasicAuthHandler.java 57.14% <0.00%> (ø)
...ain/java/com/baidu/hugegraph/define/Checkable.java 100.00% <ø> (ø)
...ava/com/baidu/hugegraph/serializer/Serializer.java 100.00% <ø> (ø)
...ugegraph/backend/cache/CachedGraphTransaction.java 85.71% <ø> (ø)
...gegraph/backend/cache/CachedSchemaTransaction.java 90.65% <ø> (ø)
...va/com/baidu/hugegraph/backend/id/IdGenerator.java 77.20% <ø> (ø)
... and 34 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@seagle-yuan seagle-yuan requested a review from javeme June 6, 2022 04:51
@seagle-yuan seagle-yuan requested a review from javeme June 6, 2022 06:33
@@ -81,7 +81,8 @@ default void setup(final Map<String, Object> config) {

@Override
default User authenticate(final Map<String, String> credentials)
throws AuthenticationException {
throws AuthenticationException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

align with final

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so we could algin with "final"
if we break line from "credentials",we can align with "final",because "final Map<String, String> credentials" are the entire content in parenthesis
but "throws" is out of parenthesis

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -34,8 +34,8 @@ public interface Records {

Id nextKey();

PathSet findPath(Id target, Function<Id, Boolean> filter,
boolean all, boolean ring);
PathSet findPath(Id target, Function<Id, Boolean> filter, boolean all,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer to wrap line before "boolean all"

Copy link
Contributor Author

@seagle-yuan seagle-yuan Jun 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line only 75 char. so i think wrap line after "boolean all" is better.

@@ -415,8 +416,8 @@ default R scan(String table, Set<byte[]> prefixes) {
/**
* Scan records by rowkey start and prefix from a table
*/
default R scan(String table, byte[] startRow,
boolean inclusiveStart, byte[] prefix) {
default R scan(String table, byte[] startRow, boolean inclusiveStart,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer to wrap line before "boolean inclusiveStart"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line only 78 char.


/**
* Add a row record to a table(can be used when adding an index)
*/
public abstract void put(String table, byte[] family,
byte[] rowkey, byte[] qualifier, byte[] value);
void put(String table, byte[] family, byte[] rowkey, byte[] qualifier,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer to wrap line before "byte[] qualifier"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line is only 79 char


/**
* Get a record by rowkey and qualifier from a table
*/
public R get(String table, byte[] family, byte[] rowkey,
R get(String table, byte[] family, byte[] rowkey,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing updating

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?


/**
* Delete a record by rowkey and qualifier from a table
*/
public default void remove(String table, byte[] family,
byte[] rowkey, byte[] qualifier) {
default void remove(String table, byte[] family, byte[] rowkey,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer to wrap line before "byte[] rowkey"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line only 72 char

@seagle-yuan seagle-yuan requested a review from javeme June 15, 2022 13:43
@@ -484,7 +483,7 @@ private int batchSize() {
}

private void checkBatchResults(Object[] results, List<Row> rows)
throws Throwable {
throws Throwable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

/**
* Session implement for HBase
*/
public class Session extends AbstractBackendSession
implements HbaseSession<RowIterator> {
implements HbaseSession<RowIterator> {
Copy link
Contributor

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: align 'implements' with 'extends'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public default User authenticate(final Map<String, String> credentials)
throws AuthenticationException {
default User authenticate(final Map<String, String> credentials)
throws AuthenticationException {
Copy link
Contributor

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: align 'throws' with 'final'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -554,7 +552,7 @@ public Integer commit() {
} catch (Throwable e) {
// TODO: Mark and delete committed records
throw new BackendException("Failed to commit, " +
"there may be inconsistent states for HBase", e);
"there may be inconsistent states for HBase", e);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exceed 100 chars?

Copy link
Contributor

@zyxxoo zyxxoo Oct 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will fixed by #1996

@@ -809,7 +807,7 @@ private void dump(String table, Scan scan) throws IOException {
byte[] key = CellUtil.cloneQualifier(cell);
byte[] val = CellUtil.cloneValue(cell);
LOG.info(" {}={}", StringEncoding.format(key),
StringEncoding.format(val));
StringEncoding.format(val));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also wrap line before StringEncoding.format(key) for logic symmetry

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -198,7 +202,6 @@
</module>
<module name="OneStatementPerLine"/>
<module name="MultipleVariableDeclarations"/>
<module name="MissingSwitchDefault"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why removed MissingSwitchDefault check

@@ -485,7 +483,7 @@ private int batchSize() {
}

private void checkBatchResults(Object[] results, List<Row> rows)
throws Throwable {
throws Throwable {
Copy link
Contributor

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: align 'throws' with 'Object[]'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@javeme
Copy link
Contributor

javeme commented Aug 5, 2022

please also address some other comments: https://github.com/apache/incubator-hugegraph/pull/1896/files

@github-actions
Copy link

github-actions bot commented Sep 4, 2022

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

Copy link
Contributor

@javeme javeme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. there are still some tiny comments to be addressed: https://github.com/apache/incubator-hugegraph/pull/1896/files

@zyxxoo zyxxoo merged commit cc80d27 into apache:master Oct 30, 2022
@javeme
Copy link
Contributor

javeme commented Oct 30, 2022

Looks good. there are still some tiny comments to be addressed: https://github.com/apache/incubator-hugegraph/pull/1896/files

will fixed by #1996

@seagle-yuan seagle-yuan deleted the checkstyle-redundant branch November 28, 2022 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants