Skip to content

[HUDI-531] Add java doc for hudi test suite general classes#1900

Merged
yanghua merged 1 commit intoapache:masterfrom
wangxianghu:HUDI-531
Aug 28, 2020
Merged

[HUDI-531] Add java doc for hudi test suite general classes#1900
yanghua merged 1 commit intoapache:masterfrom
wangxianghu:HUDI-531

Conversation

@wangxianghu
Copy link
Contributor

Tips

What is the purpose of the pull request

Add java doc for hudi test suite general classes

Brief change log

Add java doc for hudi test suite general classes

Verify this pull request

This pull request is a trivial rework / code cleanup without any test coverage.

Committer checklist

  • Has a corresponding JIRA in PR title & commit

  • Commit message is descriptive of the change

  • CI is green

  • Necessary doc changes done or have another open PR

  • For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

@wangxianghu
Copy link
Contributor Author

Hi @yanghua , please take a look when free :)

@yanghua
Copy link
Contributor

yanghua commented Aug 2, 2020

Hi @yanghua , please take a look when free :)

There is a conflict file. Pls resolve it then ping me. tks.

@yanghua yanghua self-assigned this Aug 2, 2020
@yanghua yanghua self-requested a review August 2, 2020 13:32
@codecov-commenter
Copy link

Codecov Report

Merging #1900 into master will decrease coverage by 0.69%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1900      +/-   ##
============================================
- Coverage     61.38%   60.69%   -0.70%     
+ Complexity     3799     3758      -41     
============================================
  Files           458      458              
  Lines         19573    19573              
  Branches       1959     1959              
============================================
- Hits          12015    11879     -136     
- Misses         6744     6892     +148     
+ Partials        814      802      -12     
Flag Coverage Δ Complexity Δ
#hudicli 67.63% <ø> (-1.86%) 1505.00 <ø> (-42.00)
#hudiclient 76.73% <ø> (-2.49%) 1315.00 <ø> (-42.00)
#hudicommon 55.07% <ø> (+0.04%) 1518.00 <ø> (+1.00)
#hudihadoopmr 38.99% <ø> (ø) 163.00 <ø> (ø)
#hudihivesync 72.01% <ø> (ø) 121.00 <ø> (ø)
#hudispark 49.24% <ø> (ø) 124.00 <ø> (ø)
#huditimelineservice 63.47% <ø> (ø) 47.00 <ø> (ø)
#hudiutilities 74.12% <ø> (ø) 280.00 <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
...n/java/org/apache/hudi/index/hbase/HBaseIndex.java 22.00% <0.00%> (-59.81%) 8.00% <0.00%> (-34.00%)
.../index/hbase/DefaultHBaseQPSResourceAllocator.java 55.55% <0.00%> (-44.45%) 3.00% <0.00%> (-2.00%)
...org/apache/hudi/config/HoodieHBaseIndexConfig.java 39.18% <0.00%> (-5.41%) 2.00% <0.00%> (ø%)
...java/org/apache/hudi/config/HoodieWriteConfig.java 77.25% <0.00%> (-1.45%) 88.00% <0.00%> (-4.00%)
...java/org/apache/hudi/client/HoodieWriteClient.java 75.86% <0.00%> (-0.87%) 46.00% <0.00%> (-1.00%)
...ache/hudi/common/fs/inline/InMemoryFileSystem.java 89.65% <0.00%> (+10.34%) 16.00% <0.00%> (+1.00%)

@wangxianghu
Copy link
Contributor Author

Hi @yanghua , please take a look when free :)

There is a conflict file. Pls resolve it then ping me. tks.

@yanghua thanks for your review, it is ready now

Copy link
Contributor

@yanghua yanghua left a comment

Choose a reason for hiding this comment

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

@Mathieu1124 Thanks for your contribution. Left some comments. IMO, you should add more information to the doc of the classes.

Copy link
Contributor

Choose a reason for hiding this comment

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

bulkInsert Node -> bulk insert node?

Copy link
Contributor

Choose a reason for hiding this comment

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

Node -> node?

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.

ditto

Copy link
Contributor

Choose a reason for hiding this comment

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

DagScheduler -> DAG scheduler

Copy link
Contributor

Choose a reason for hiding this comment

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

workflowDags -> workflow DAGs

Copy link
Contributor

Choose a reason for hiding this comment

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

execute -> executing.

@wangxianghu
Copy link
Contributor Author

@Mathieu1124 Thanks for your contribution. Left some comments. IMO, you should add more information to the doc of the classes.

@yanghua Thanks for your detailed review, I will enrich and improve the docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

execute -> execution

Copy link
Contributor

Choose a reason for hiding this comment

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

Execution -> execution.

@wangxianghu wangxianghu force-pushed the HUDI-531 branch 5 times, most recently from ec52824 to 7ded665 Compare August 10, 2020 06:43
@wangxianghu
Copy link
Contributor Author

@yanghua this pr is ready for review now :)

Copy link
Contributor

@yanghua yanghua left a comment

Choose a reason for hiding this comment

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

@wangxianghu Left some comments.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, we do not need this comment.

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.

we may not change this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why we should change these comment styles for fields?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why we should change these comment styles for fields?

my bad, that`s the coding guidelines of Alibaba.
rolled back already :)

Copy link
Contributor

@yanghua yanghua left a comment

Choose a reason for hiding this comment

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

@wangxianghu LGTM.

@yanghua yanghua changed the title [HUDI-531]Add java doc for hudi test suite general classes [HUDI-531] Add java doc for hudi test suite general classes Aug 28, 2020
@yanghua yanghua merged commit fa81248 into apache:master Aug 28, 2020
@wangxianghu wangxianghu deleted the HUDI-531 branch August 28, 2020 10:14
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