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 hugegraph-hbase and hugegraph-example checkstyle issue #1864

Merged
merged 26 commits into from
May 9, 2022

Conversation

mayelena
Copy link
Contributor

@mayelena mayelena commented May 7, 2022

No description provided.

@mayelena mayelena changed the title fix hugegraph-hbase checkstyle issue fix hugegraph-hbase and hugegraph-example checkstyle issue May 7, 2022
@@ -283,7 +283,8 @@ protected BackendEntryIterator newEntryIterator(Query query,
});
}

protected void parseRowColumns(Result row, BackendEntry entry, Query query, boolean enablePartition)
protected void parseRowColumns(Result row, BackendEntry entry, Query query,
boolean enablePartition)
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 "Result"

@@ -54,7 +54,7 @@ public abstract class HbaseStore extends AbstractBackendStore<Session> {

private static final Logger LOG = Log.logger(HbaseStore.class);

private final BackendFeatures FEATURES;
private final BackendFeatures features;
Copy link
Contributor

Choose a reason for hiding this comment

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

also update references

@@ -84,6 +86,7 @@
import com.google.common.util.concurrent.Futures;

public class HbaseSessions extends BackendSessionPool {
private static final Logger LOG = Log.logger(HbaseSessions.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

add a blank line after class define

TableDescriptorBuilder builder = TableDescriptorBuilder.newBuilder(
TableName.valueOf(this.namespace, table));
for (byte[] cf : cfs) {
builder.setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(cf)
.build());
}
byte[][] splits = new byte[numOfPartitions - 1][org.apache.hadoop.hbase.util.Bytes.SIZEOF_SHORT];
byte[][] splits = new byte[numOfPartitions - 1][org.apache.hadoop
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 "[org"

@@ -793,18 +798,18 @@ public long storeSize(String table) throws IOException {
*/
@SuppressWarnings("unused")
private void dump(String table, Scan scan) throws IOException {
System.out.println(String.format(">>>> scan table %s with %s",
LOG.info(String.format(">>>> scan table %s with %s",
Copy link
Contributor

Choose a reason for hiding this comment

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

LOG.info(">>>> scan table {} with {}",

CellScanner cellScanner = row.cellScanner();
while (cellScanner.advance()) {
Cell cell = cellScanner.current();
byte[] key = CellUtil.cloneQualifier(cell);
byte[] val = CellUtil.cloneValue(cell);
System.out.println(String.format(" %s=%s",
LOG.info(String.format(" %s=%s",
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@@ -445,7 +447,8 @@ public static class HbaseSchemaStore extends HbaseStore {

public HbaseSchemaStore(HugeConfig config, BackendStoreProvider provider,
String namespace, String store) {
super(provider, namespace, store, config.get(HbaseOptions.HBASE_ENABLE_PARTITION).booleanValue());
super(provider, namespace, store, config.get(HbaseOptions
Copy link
Contributor

Choose a reason for hiding this comment

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

wrap line before config

@@ -492,7 +495,8 @@ public static class HbaseGraphStore extends HbaseStore {
private boolean enablePartition;
public HbaseGraphStore(HugeConfig config, BackendStoreProvider provider,
String namespace, String store) {
super(provider, namespace, store, config.get(HbaseOptions.HBASE_ENABLE_PARTITION).booleanValue());
super(provider, namespace, store, config.get(HbaseOptions.HBASE_ENABLE_PARTITION)
Copy link
Contributor

Choose a reason for hiding this comment

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

wrap line before config

@@ -30,7 +30,7 @@ public class ThreadRangePerfTest {

public static void main(String[] args) throws Exception {
if (args.length != 6) {
System.out.println("Usage: minThread maxThread threadStep " +
LOG.info("Usage: minThread maxThread threadStep " +
Copy link
Contributor

Choose a reason for hiding this comment

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

update alignment of line 34

LOG.info("Insert rate with threads: {} vertices/s & {} edges/s, " +
"insert total {} vertices & {} edges, cost time: {}ms",
LOG.info("Insert rate with threads: {} vertices/s & {} edges/s,
insert total {} vertices & {} edges, cost time: {}ms",
Copy link
Contributor

Choose a reason for hiding this comment

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

there is a syntax error here, please keep the origin code

LOG.info("Query rate with threads: {} vertices/s, " +
"query total vertices {}, cost time: {}ms",
LOG.info("Query rate with threads: {} vertices/s,
query total vertices {}, cost time: {}ms",
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto


// query edges by condition
List<Edge> tomhanksMovies = graph.traversal().V()
.hasLabel("person")
.has("name", "Tom Hanks")
.outE("ACTED_IN").toList();
System.out.println(">>>> Tom Hanks ACTED_IN: " + tomhanksMovies);
LOG.info{(">>>> Tom Hanks ACTED_IN: {}", tomhanksMovies);
Copy link
Contributor

Choose a reason for hiding this comment

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

remove "{"


G.addEdge("directedBy", M);
G.addEdge("directedBy", m);
Copy link
Contributor

Choose a reason for hiding this comment

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

Error: /home/runner/work/incubator-hugegraph/incubator-hugegraph/hugegraph-example/src/main/java/com/baidu/hugegraph/example/Example3.java:[122,9] cannot find symbol
symbol: variable G
location: class com.baidu.hugegraph.example.Example3

javeme
javeme previously approved these changes May 8, 2022
@codecov
Copy link

codecov bot commented May 8, 2022

Codecov Report

Merging #1864 (3b171c3) into master (6328b8d) will increase coverage by 3.99%.
The diff coverage is 75.00%.

@@             Coverage Diff              @@
##             master    #1864      +/-   ##
============================================
+ Coverage     66.96%   70.95%   +3.99%     
  Complexity      980      980              
============================================
  Files           447      447              
  Lines         38045    38072      +27     
  Branches       5391     5401      +10     
============================================
+ Hits          25477    27015    +1538     
+ Misses        10065     8410    -1655     
- Partials       2503     2647     +144     
Impacted Files Coverage Δ
...aidu/hugegraph/backend/store/hbase/HbaseTable.java 75.92% <ø> (ø)
...idu/hugegraph/backend/store/hbase/HbaseTables.java 90.00% <ø> (ø)
...u/hugegraph/backend/store/hbase/HbaseSessions.java 60.00% <50.00%> (+0.05%) ⬆️
...aidu/hugegraph/backend/store/hbase/HbaseStore.java 74.05% <100.00%> (+0.21%) ⬆️
...du/hugegraph/traversal/optimize/HugeCountStep.java 61.11% <0.00%> (-30.56%) ⬇️
...u/hugegraph/traversal/optimize/HugeVertexStep.java 67.81% <0.00%> (-6.87%) ⬇️
...du/hugegraph/traversal/optimize/HugeGraphStep.java 75.58% <0.00%> (-6.47%) ⬇️
...va/com/baidu/hugegraph/structure/HugeProperty.java 48.00% <0.00%> (-4.00%) ⬇️
...va/com/baidu/hugegraph/util/collection/IntSet.java 73.72% <0.00%> (-1.28%) ⬇️
.../java/com/baidu/hugegraph/auth/RolePermission.java 90.69% <0.00%> (-1.07%) ⬇️
... and 63 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 6328b8d...3b171c3. Read the comment docs.

Copy link
Member

@imbajin imbajin left a comment

Choose a reason for hiding this comment

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

@seagle-yuan if we don't divide these case, will it hit against the check-style?

Comment on lines 239 to 240
builder.setColumnFamily(ColumnFamilyDescriptorBuilder.newBuilder(cf)
.build());
Copy link
Member

Choose a reason for hiding this comment

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

align or not to divide?

Copy link
Contributor

Choose a reason for hiding this comment

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

No rules will be hit.
The below line is indented more than 4 blankspace.
And we setted forceStrictCondition = false, so more than 4 space is ok.

https://checkstyle.sourceforge.io/config_misc.html#Indentation.

there isn't any rules about align in all checkstyle rules

https://checkstyle.sourceforge.io/checks.html

Comment on lines +242 to +243
byte[][] splits = new byte[numOfPartitions - 1]
[org.apache.hadoop.hbase.util.Bytes.SIZEOF_SHORT];
Copy link
Member

@imbajin imbajin May 9, 2022

Choose a reason for hiding this comment

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

seems divide it makes a little strange for readable

Copy link
Contributor

Choose a reason for hiding this comment

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

it rely on the setting of IDE

CellScanner cellScanner = row.cellScanner();
while (cellScanner.advance()) {
Cell cell = cellScanner.current();
byte[] key = CellUtil.cloneQualifier(cell);
byte[] val = CellUtil.cloneValue(cell);
System.out.println(String.format(" %s=%s",
LOG.info(String.format(" {}={}",
Copy link
Contributor

Choose a reason for hiding this comment

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

remove String.format and update alignment

@imbajin imbajin merged commit cf767dc into apache:master May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants