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

[refactor] #4315: split pipeline events #4366

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

mversic
Copy link
Contributor

@mversic mversic commented Mar 14, 2024

Description

  • introduced WithEvents which makes sure that events are consumed upon block state transition
  • refactored PipelineEvent to a more sane representation

Linked issue

Closes #4315

Benefits

Checklist

  • I've read CONTRIBUTING.md
  • I've used the standard signed-off commit format (or will squash just before merging)
  • All applicable CI checks pass (or I promised to make them pass later)
  • (optional) I've written unit tests for the code changes
  • I replied to all comments after code review, marking all implemented changes with thumbs up

@github-actions github-actions bot added Refactor Improvement to overall code quality iroha2-dev The re-implementation of a BFT hyperledger in RUST labels Mar 14, 2024
@mversic mversic force-pushed the split_pipeline_events branch 6 times, most recently from 59c27fb to 94aa11e Compare March 14, 2024 07:56
@github-actions github-actions bot added the api-changes Changes in the API for client libraries label Mar 14, 2024
@mversic mversic marked this pull request as ready for review March 14, 2024 07:57
@mversic mversic force-pushed the split_pipeline_events branch 2 times, most recently from 4b628a6 to 8b760fe Compare March 14, 2024 09:08
@DCNick3
Copy link
Contributor

DCNick3 commented Mar 14, 2024

Can merging this please be postponed to after #4240 lands? I don't want to rebase it again..

@mversic mversic force-pushed the split_pipeline_events branch 2 times, most recently from 5f60347 to 52ee097 Compare March 14, 2024 10:20
@mversic mversic force-pushed the split_pipeline_events branch 2 times, most recently from c010136 to b097945 Compare March 19, 2024 07:26
@mversic mversic marked this pull request as ready for review March 22, 2024 06:31
@mversic mversic force-pushed the split_pipeline_events branch 11 times, most recently from 2dcde0e to f87bc28 Compare March 25, 2024 11:05
Erigara
Erigara previously approved these changes Mar 25, 2024
Copy link
Contributor

@Erigara Erigara left a comment

Choose a reason for hiding this comment

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

This PR is a lot semi related changes, consider to make separate PR for them in the future, it will make it easier to review.

Other than that look good.

@mversic mversic force-pushed the split_pipeline_events branch 4 times, most recently from 576dcc0 to 1815470 Compare March 27, 2024 08:39
@coveralls
Copy link

coveralls commented Mar 27, 2024

Pull Request Test Coverage Report for Build 8455027251

Details

  • 509 of 853 (59.67%) changed or added relevant lines in 31 files are covered.
  • 4945 unchanged lines in 94 files lost coverage.
  • Overall coverage increased (+1.0%) to 57.744%

Changes Missing Coverage Covered Lines Changed/Added Lines %
client/src/http.rs 2 3 66.67%
core/src/smartcontracts/wasm.rs 0 1 0.0%
data_model/src/block.rs 6 7 85.71%
data_model/src/events/data/filters.rs 0 1 0.0%
data_model/src/transaction.rs 7 8 87.5%
telemetry/src/dev.rs 0 1 0.0%
core/src/kura.rs 1 3 33.33%
core/src/queue.rs 86 88 97.73%
core/src/smartcontracts/isi/triggers/set.rs 4 6 66.67%
core/src/state.rs 29 31 93.55%
Files with Coverage Reduction New Missed Lines %
primitives/src/conststr.rs 1 91.14%
crypto/src/hash.rs 1 73.78%
ffi/derive/src/convert.rs 1 84.45%
primitives/src/lib.rs 1 0.0%
core/src/sumeragi/network_topology.rs 1 98.78%
data_model/derive/src/model.rs 1 81.0%
crypto/src/signature/bls/mod.rs 2 0.0%
config/src/snapshot.rs 3 78.57%
data_model/derive/src/has_origin.rs 3 95.16%
config/src/kura.rs 3 80.0%
Totals Coverage Status
Change from base Build 7884695009: 1.0%
Covered Lines: 23351
Relevant Lines: 40439

💛 - Coveralls

client/src/client.rs Outdated Show resolved Hide resolved
data_model/src/events/mod.rs Outdated Show resolved Hide resolved
data_model/src/events/pipeline.rs Show resolved Hide resolved
data_model/src/events/pipeline.rs Show resolved Hide resolved
data_model/src/events/data/filters.rs Show resolved Hide resolved
data_model/derive/src/model.rs Show resolved Hide resolved
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
@mversic mversic merged commit e7e24db into hyperledger-iroha:iroha2-dev Mar 28, 2024
15 checks passed
@mversic mversic deleted the split_pipeline_events branch March 28, 2024 07:42
@timofeevmd timofeevmd added the QA-confirmed This bug is reproduced and needs a fix label Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-changes Changes in the API for client libraries config-changes Changes in configuration and start up of the Iroha iroha2-dev The re-implementation of a BFT hyperledger in RUST QA-confirmed This bug is reproduced and needs a fix Refactor Improvement to overall code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants