-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
inject CorruptedDataFilesHandler instead of making it static #3107
inject CorruptedDataFilesHandler instead of making it static #3107
Conversation
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.
utACK
@@ -137,7 +138,7 @@ public DisputeManager(P2PService p2PService, | |||
OpenOfferManager openOfferManager, | |||
KeyRing keyRing, | |||
PersistenceProtoResolver persistenceProtoResolver, | |||
@Named(Storage.STORAGE_DIR) File storageDir) { | |||
@Named(Storage.STORAGE_DIR) File storageDir, CorruptedDatabaseFilesHandler corruptedDatabaseFilesHandler) { |
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.
Can you add a line break. So params are better separated/readable?
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.
sure, i wonder if there is a way to make idea do it automatically.
Just a small thing regarding commit messages. Please start them with uppercase https://chris.beams.io/posts/git-commit/ |
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.
utACK - But please see my comment for future commits.
just a small rabbit-hole that i went down while looking at the code.
storage can probably be injected directly in more places but i didn't want to change any logic in this pr.