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

Expose identity earnings to SSE state #1916

Merged
merged 3 commits into from
Mar 26, 2020
Merged

Conversation

Waldz
Copy link
Member

@Waldz Waldz commented Mar 25, 2020

Updates: #1731

Web UI now can interactively show increasing earnings

@Waldz Waldz force-pushed the feature/expose-provider-earnings branch 4 times, most recently from b9bb8b7 to 028757d Compare March 25, 2020 20:46
core/state/state.go Show resolved Hide resolved
core/state/state.go Outdated Show resolved Hide resolved
}

// GetBalance returns a pre-defined balance.
func (mbp *mockBalanceProvider) GetBalance(_ identity.Identity) uint64 {
Copy link
Member

Choose a reason for hiding this comment

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

This is identical to mocks.BalanceProvider

Copy link
Member Author

Choose a reason for hiding this comment

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

I moved mocks next to tests, because keeping them global packages causes circular dependencies: pingpong -> mocks -> connections -> pingpong

Copy link
Member

Choose a reason for hiding this comment

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

There is no dependency on connections in the mocks package. Either way, mocks should have minimal dependencies.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added similar mock EarningsProvider.SettlementState(_ identity.Identity) pingpong.SettlementState and mocks package starts to be interdependent.
Lets agree on having mocks next to package we're testing.

Copy link
Member

Choose a reason for hiding this comment

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

Such widely used types as identity.Identity or in this case pingpong.SettlementState could be in a leaf package with minimal dependencies - that will solve the package cycle. If it's a one-off mock - then it's good to have it next to the test. If it's used multiple times, then the maintenance becomes a burden and it should be in mocks.

@Waldz Waldz force-pushed the feature/expose-provider-earnings branch 2 times, most recently from b3f26f1 to ffeb620 Compare March 26, 2020 08:09
@Waldz Waldz requested a review from tadaskay March 26, 2020 08:09
@Waldz Waldz force-pushed the feature/expose-provider-earnings branch from ffeb620 to 59f21cc Compare March 26, 2020 10:56
@Waldz Waldz merged commit 27a4e3d into master Mar 26, 2020
@Waldz Waldz deleted the feature/expose-provider-earnings branch March 26, 2020 11:38
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.

None yet

3 participants