-
Notifications
You must be signed in to change notification settings - Fork 24
refactor: changes to dips to better support payment collection in indexer-agent #608
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
Conversation
Pull Request Test Coverage Report for Build 13208353292Details
💛 - Coveralls |
d8b88eb to
cfb8358
Compare
8b139f7 to
845f6f3
Compare
845f6f3 to
ecd0185
Compare
LNSD
left a comment
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.
LGTM ✅
| package graphprotocol.indexer.dips; | ||
|
|
||
| service DipsService { | ||
| service IndexerDipsService { |
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.
I would suggest naming it GatewayDipsService.
| service IndexerDipsService { | |
| service GatewayDipsService { |
I want to rename the "Dipper" to DIPs Gateway at some point in the future.
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.
I imagine you mean in DipperService, rather than here, right?
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| pub mod cost_model; | ||
| pub mod dips; |
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.
You removed dips from the modules but didn't remove dips.rs file. Is there any reason?
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.
I moved it to database.rs on the dips crate, so it shows in git as moved rather than removed
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.
You are right! Github tricked me
ecd0185 to
fa1c479
Compare
gusinacio
left a comment
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.
This PR affects directly DIPs with no changes in the rest of the application. LGTM
Uh oh!
There was an error while loading. Please reload this page.