-
Notifications
You must be signed in to change notification settings - Fork 3
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
refactor: use commons module in simulator #271
Conversation
a44ffcc
to
db7654d
Compare
7b202ca
to
76efcc6
Compare
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.
LG, just 1 question/nit.
common/src/main/java/com/hedera/block/common/utils/FileUtilities.java
Outdated
Show resolved
Hide resolved
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.
It appears you ran an IDE reformat on these files without disabling a lot of very bad defaults. It's important to keep in mind that most IDEs do not, and often cannot, correctly format the codebase. The gradle spotlessApply
task is much preferred.
Most of these comments are to not apply those reformats (particularly in Javadoc which is using a very resource-expensive, completely unnecessary, and relatively difficult to read formatting pattern).
Beyond that the Constants file should not have been removed (it holds exactly the constants we should be keeping and using from the Simulator).
common/src/main/java/com/hedera/block/common/utils/FileUtilities.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/hedera/block/common/utils/FileUtilities.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/hedera/block/common/utils/FileUtilities.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/hedera/block/common/utils/FileUtilities.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/hedera/block/common/utils/FileUtilities.java
Outdated
Show resolved
Hide resolved
simulator/src/main/java/com/hedera/block/simulator/BlockStreamSimulator.java
Outdated
Show resolved
Hide resolved
simulator/src/main/java/com/hedera/block/simulator/BlockStreamSimulator.java
Outdated
Show resolved
Hide resolved
simulator/src/main/java/com/hedera/block/simulator/Constants.java
Outdated
Show resolved
Hide resolved
simulator/src/main/java/com/hedera/block/simulator/generator/BlockAsDirBlockStreamManager.java
Outdated
Show resolved
Hide resolved
simulator/src/main/java/com/hedera/block/simulator/generator/BlockAsDirBlockStreamManager.java
Outdated
Show resolved
Hide resolved
simulator/src/main/java/com/hedera/block/simulator/generator/BlockAsFileLargeDataSets.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Atanas Atanasov <[email protected]>
Signed-off-by: Atanas Atanasov <[email protected]>
…ier usage and spotlessApply Signed-off-by: Atanas Atanasov <[email protected]>
Signed-off-by: Atanas Atanasov <[email protected]>
Signed-off-by: Atanas Atanasov <[email protected]>
Signed-off-by: Atanas Atanasov <[email protected]>
Signed-off-by: Atanas Atanasov <[email protected]>
6811957
to
e2923e0
Compare
Signed-off-by: Atanas Atanasov <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #271 +/- ##
============================================
+ Coverage 99.62% 99.81% +0.18%
+ Complexity 288 285 -3
============================================
Files 57 56 -1
Lines 1060 1053 -7
Branches 78 77 -1
============================================
- Hits 1056 1051 -5
+ Misses 3 2 -1
+ Partials 1 0 -1
|
common/src/test/java/com/hedera/block/common/utils/FileUtilitiesTest.java
Show resolved
Hide resolved
simulator/src/main/java/com/hedera/block/simulator/generator/BlockAsFileLargeDataSets.java
Outdated
Show resolved
Hide resolved
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.
Joseph has a few comments but otherwise looks good
simulator/src/main/java/com/hedera/block/simulator/generator/BlockAsFileLargeDataSets.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Atanas Atanasov <[email protected]>
simulator/src/main/java/com/hedera/block/simulator/generator/BlockAsFileLargeDataSets.java
Show resolved
Hide resolved
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.
Looks good
Description:
:simulator
to start using logic from:common
Related issue(s):
Fixes #253
Notes for reviewer:
Checklist
com.hedera.block.simulator.Constants
from:simulator
and replace usages withcom.hedera.block.common.constants.StringsConstants
from:common
com.hedera.block.simulator.generator.Utils
from:simulator
and replace usages withcom.hedera.block.common.utils.FileUtilities
from:common