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

feat: Add Dagger for Dependency Injection Framework #118

Merged
merged 12 commits into from
Aug 23, 2024
Merged

Conversation

AlfredoG87
Copy link
Contributor

@AlfredoG87 AlfredoG87 commented Aug 21, 2024

Description:

  • Adds Gradle Build dependencies
  • Refactors Server and moves Everything away to BlockNodeApp
  • Refactors necessary to Inject first 2 dependencies
    • ServiceStatus
    • HealthService
    • BlockNodeContext

Related issue(s):

Fixes #110

Notes for reviewer:

We did not perform a Unit test for new class BlockNodeApp since we need to wait for all other dependencies to be injected using Dagger, so we can mock them and also inject mocks when doing a UT for that class.

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@AlfredoG87 AlfredoG87 self-assigned this Aug 21, 2024
@AlfredoG87 AlfredoG87 added the Improvement Code changes driven by non business requirements label Aug 21, 2024
@AlfredoG87 AlfredoG87 added this to the 0.1.0 milestone Aug 21, 2024
@AlfredoG87 AlfredoG87 marked this pull request as ready for review August 21, 2024 20:37
@AlfredoG87 AlfredoG87 requested a review from a team as a code owner August 21, 2024 20:37
Copy link
Member

@jsync-swirlds jsync-swirlds left a comment

Choose a reason for hiding this comment

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

A few items that could be improved, and a couple of questions.

jsync-swirlds
jsync-swirlds previously approved these changes Aug 23, 2024
Signed-off-by: Alfredo Gutierrez <[email protected]>
Signed-off-by: Alfredo Gutierrez <[email protected]>
Signed-off-by: Alfredo Gutierrez <[email protected]>
Signed-off-by: Alfredo Gutierrez <[email protected]>
…UnhandledIOException that is a runtime wrapper for the checked IOException

Signed-off-by: Alfredo Gutierrez <[email protected]>
Signed-off-by: Alfredo Gutierrez <[email protected]>
Signed-off-by: Alfredo Gutierrez <[email protected]>
@AlfredoG87
Copy link
Contributor Author

Last Changes was only rebase and conflict resolutions, in jpms gradle file, Server.java, BlockNodeApp.java files.

Copy link
Contributor

@mattp-swirldslabs mattp-swirldslabs left a comment

Choose a reason for hiding this comment

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

LGTM, we'll fix the codecov in part 2

@AlfredoG87 AlfredoG87 merged commit d53758b into main Aug 23, 2024
6 of 7 checks passed
@AlfredoG87 AlfredoG87 deleted the di-dagger2 branch August 23, 2024 21:40
Copy link

codecov bot commented Aug 23, 2024

Codecov Report

Attention: Patch coverage is 3.03030% with 32 lines in your changes missing coverage. Please review.

Project coverage is 93.90%. Comparing base (d938806) to head (08f8150).
Report is 2 commits behind head on main.

Files Patch % Lines
...ain/java/com/hedera/block/server/BlockNodeApp.java 0.00% 30 Missing ⚠️
...dera/block/server/BlockNodeAppInjectionModule.java 33.33% 2 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##                main     #118      +/-   ##
=============================================
- Coverage     100.00%   93.90%   -6.10%     
- Complexity       121      122       +1     
=============================================
  Files             21       23       +2     
  Lines            492      525      +33     
  Branches          27       27              
=============================================
+ Hits             492      493       +1     
- Misses             0       32      +32     
Files Coverage Δ
...ava/com/hedera/block/server/ServiceStatusImpl.java 100.00% <ø> (ø)
.../hedera/block/server/health/HealthServiceImpl.java 100.00% <ø> (ø)
...dera/block/server/BlockNodeAppInjectionModule.java 33.33% <33.33%> (ø)
...ain/java/com/hedera/block/server/BlockNodeApp.java 0.00% <0.00%> (ø)

@AlfredoG87 AlfredoG87 mentioned this pull request Aug 24, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Code changes driven by non business requirements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Dagger as DI in codebase
3 participants