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

More events on renderers #1000

Merged
merged 3 commits into from
Sep 14, 2020
Merged

More events on renderers #1000

merged 3 commits into from
Sep 14, 2020

Conversation

dahlia
Copy link
Contributor

@dahlia dahlia commented Sep 11, 2020

Added two more render methods:

  • IActionRenderer<T>.RenderBlockEnd()
  • IRenderer<T>.RenderReorgEnd()

See also the docs on IRenderer<T>:

IRenderer<T>

… and IActionRenderer<T>:

IActionRenderer<T>

By utilizing RenderBlockEnd() event, DelayedActionRenderer<T> became to atomically update its internal buffers (it's further than the approach made in #998).

I also refactored DumRenderer<T> (in the tests project) into RecordingRenderer<T> to reorganize RenderRecord into hierarchical layers.

Reorganized RenderRecord into hierarchical layers.

[changelog skip]
@codecov
Copy link

codecov bot commented Sep 11, 2020

Codecov Report

Merging #1000 into main will increase coverage by 0.10%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1000      +/-   ##
==========================================
+ Coverage   88.98%   89.08%   +0.10%     
==========================================
  Files         328      328              
  Lines       29481    29702     +221     
==========================================
+ Hits        26233    26460     +227     
+ Misses       1694     1684      -10     
- Partials     1554     1558       +4     
Impacted Files Coverage Δ
Libplanet.Tests/Blockchain/BlockChainTest.cs 98.57% <ø> (+0.01%) ⬆️
...lockchain/Renderers/AnonymousActionRendererTest.cs 88.39% <ø> (+0.81%) ⬆️
...ests/Blockchain/Renderers/AnonymousRendererTest.cs 84.31% <ø> (+0.44%) ⬆️
.../Blockchain/Renderers/DelayedActionRendererTest.cs 94.35% <ø> (+0.39%) ⬆️
....Tests/Blockchain/Renderers/DelayedRendererTest.cs 95.32% <ø> (+0.19%) ⬆️
...s/Blockchain/Renderers/LoggedActionRendererTest.cs 99.30% <ø> (+0.03%) ⬆️
...t.Tests/Blockchain/Renderers/LoggedRendererTest.cs 99.15% <ø> (+0.03%) ⬆️
Libplanet.Tests/Common/RecordingRenderer.cs 90.12% <ø> (ø)
Libplanet.Tests/Common/RenderRecord.cs 100.00% <ø> (ø)
Libplanet.Tests/Net/SwarmTest.Fixtures.cs 100.00% <ø> (ø)
... and 30 more

Copy link
Member

@limebell limebell left a comment

Choose a reason for hiding this comment

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

Newly introduced events should be listed on the CHANGES.md.

longfin
longfin previously approved these changes Sep 14, 2020
earlbread
earlbread previously approved these changes Sep 14, 2020
moreal
moreal previously approved these changes Sep 14, 2020
@dahlia
Copy link
Contributor Author

dahlia commented Sep 14, 2020

Newly introduced events should be listed on the CHANGES.md.

@limebell As IRenderer<T> & IActionRenderer<T> have never been released yet, I didn't list them on the changelog.

riemannulus
riemannulus previously approved these changes Sep 14, 2020
limebell
limebell previously approved these changes Sep 14, 2020
@limebell
Copy link
Member

limebell commented Sep 14, 2020

I guess we should add it here at least?

Extracted rendering methods from IAction to IRenderer, which is introduced in this version. From now on, rendering logic needs to be injected from outside through BlockChain.Renderers, rather than IActions knowing how to render themselves. IRenderer also unified BlockChain.TipChanged event, and introduced new events like IRenderer.RenderActionError() and IRenderer.RenderReorg(). [#860, #875, #959, #963]

@dahlia
Copy link
Contributor Author

dahlia commented Sep 14, 2020

I guess we should add it here at least?

@limebell Okay, I'm going to add it there.

@dahlia
Copy link
Contributor Author

dahlia commented Sep 14, 2020

@planetarium/libplanet Added missing unit tests and changelogs! Please review this again.

@dahlia dahlia requested a review from moreal September 14, 2020 06:13
@dahlia dahlia merged commit 645cd4d into planetarium:main Sep 14, 2020
dahlia added a commit to dahlia/lib9c that referenced this pull request Sep 14, 2020
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.

6 participants