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] #4259: Make Action and Filter non-generic types #4375

Merged
merged 2 commits into from
Mar 19, 2024

Conversation

DCNick3
Copy link
Contributor

@DCNick3 DCNick3 commented Mar 19, 2024

Description

Make the Action and Filter types exported from iroha_data_model non-generic. This is used by code in iroha_core, so make generic copies in there. There's no need to complicate the public API due to the requirements of the implementation.

This also makes the Action constructor accept impl Into<TriggeringEventFilterBox> as a filter, simplifying most API invocations. Additional shorthand conversions from concrete event filter types are added (for example AccountEventFilter -> TriggeringEventFilterBox) to reduce the required boilerplate even more.

Linked issue

Closes #4259

Benefits

No unnecessary generic in the API -> better code completion and easier API.

Checklist

  • make CI pass

@github-actions github-actions bot added iroha2-dev The re-implementation of a BFT hyperledger in RUST api-changes Changes in the API for client libraries labels Mar 19, 2024
@DCNick3 DCNick3 changed the title Make Action and Filter non-generic types [refactor] #4259: Make Action and Filter non-generic types Mar 19, 2024
@github-actions github-actions bot added the Refactor Improvement to overall code quality label Mar 19, 2024
@DCNick3 DCNick3 force-pushed the non-generic-action branch 2 times, most recently from f7bb96f to c573b05 Compare March 19, 2024 11:52
Erigara
Erigara previously approved these changes Mar 19, 2024
@coveralls
Copy link
Collaborator

coveralls commented Mar 19, 2024

Pull Request Test Coverage Report for Build 8344105879

Details

  • 115 of 189 (60.85%) changed or added relevant lines in 12 files are covered.
  • 4571 unchanged lines in 88 files lost coverage.
  • Overall coverage increased (+0.7%) to 57.473%

Changes Missing Coverage Covered Lines Changed/Added Lines %
core/src/smartcontracts/isi/triggers/mod.rs 0 1 0.0%
data_model/src/events/data/events.rs 0 1 0.0%
data_model/src/lib.rs 0 1 0.0%
data_model/src/trigger.rs 6 7 85.71%
data_model/src/isi.rs 1 5 20.0%
smart_contract/executor/derive/src/default.rs 0 4 0.0%
smart_contract/executor/src/default.rs 0 4 0.0%
core/src/smartcontracts/isi/triggers/set.rs 25 42 59.52%
core/src/smartcontracts/isi/triggers/specialized.rs 72 113 63.72%
Files with Coverage Reduction New Missed Lines %
primitives/src/conststr.rs 1 91.14%
ffi/derive/src/convert.rs 1 84.45%
primitives/src/lib.rs 1 0.0%
core/src/sumeragi/network_topology.rs 1 98.78%
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%
primitives/src/addr.rs 3 64.04%
config/src/logger.rs 5 72.73%
Totals Coverage Status
Change from base Build 7884695009: 0.7%
Covered Lines: 22796
Relevant Lines: 39664

💛 - Coveralls

They were previously generic over the event filter type, but this was used only in iroha_core. Instead generic copies of these types are put into core, ultimetely simplifying the public API.

Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>
Make Action's constructor accept `impl Into<TriggeringEventFilterBox>`. Add shorthand conversions from concrete data event filter types into the top-level boxes

Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>
@mversic mversic merged commit 6ac5890 into hyperledger:iroha2-dev Mar 19, 2024
15 checks passed
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 iroha2-dev The re-implementation of a BFT hyperledger in RUST Refactor Improvement to overall code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants