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] #3822, #3737, #2437: Refactor iroha_data_model_derive #3897

Merged
merged 9 commits into from
Sep 26, 2023

Conversation

DCNick3
Copy link
Contributor

@DCNick3 DCNick3 commented Sep 18, 2023

Description

Introduces various improvements to iroha_data_model_derive:

[1] I am a bit unhappy with the testing of derive(Filter), but it seems so intertwined with all the parts of iroha_data_model that it's really hard to test. Additionally, testing the generated Filter impl requires the transparent_api feature

Linked issue

Closes #3882, #3737, #2437

Checklist

  • make CI pass

@github-actions github-actions bot added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Sep 18, 2023
@DCNick3 DCNick3 changed the title Rafactor iroha_data_model_derive [refactor] #3822, #3737, #2437: Refactor iroha_data_model_derive Sep 18, 2023
@coveralls
Copy link

coveralls commented Sep 18, 2023

Pull Request Test Coverage Report for Build 6308478360

  • 367 of 447 (82.1%) changed or added relevant lines in 11 files are covered.
  • 6108 unchanged lines in 102 files lost coverage.
  • Overall coverage decreased (-1.1%) to 58.3%

Changes Missing Coverage Covered Lines Changed/Added Lines %
data_model/derive/src/partially_tagged/resolve_self.rs 34 36 94.44%
data_model/derive/src/has_origin.rs 25 28 89.29%
data_model/src/predicate.rs 0 3 0.0%
data_model/derive/src/partially_tagged/mod.rs 39 43 90.7%
macro/utils/src/emitter.rs 2 6 33.33%
data_model/derive/src/lib.rs 38 44 86.36%
data_model/derive/src/filter.rs 116 124 93.55%
data_model/derive/src/model.rs 19 27 70.37%
data_model/derive/src/id.rs 54 68 79.41%
macro/utils/src/lib.rs 38 66 57.58%
Files with Coverage Reduction New Missed Lines %
cli/src/main.rs 1 0.0%
cli/src/torii/pagination.rs 1 98.9%
config/base/src/runtime_upgrades.rs 1 51.72%
config/src/torii.rs 1 96.67%
core/src/smartcontracts/isi/block.rs 1 87.5%
crypto/src/merkle.rs 1 96.23%
data_model/derive/src/id.rs 1 86.11%
data_model/src/domain.rs 1 47.37%
logger/src/lib.rs 1 95.15%
config/src/lib.rs 2 0.0%
Totals Coverage Status
Change from base Build 5423219773: -1.1%
Covered Lines: 21435
Relevant Lines: 36767

💛 - Coveralls

data_model/derive/tests/partial_tagged_serde.rs Outdated Show resolved Hide resolved
data_model/derive/tests/partial_tagged_serde.rs Outdated Show resolved Hide resolved
macro/utils/src/emitter.rs Outdated Show resolved Hide resolved
macro/utils/src/lib.rs Outdated Show resolved Hide resolved
data_model/derive/tests/filter.rs Outdated Show resolved Hide resolved
data_model/derive/src/id.rs Outdated Show resolved Hide resolved
data_model/derive/tests/id_eq_ord_hash.rs Outdated Show resolved Hide resolved
@ilchu ilchu self-assigned this Sep 19, 2023
@DCNick3 DCNick3 force-pushed the syn2-iroha-data-model branch 3 times, most recently from df72005 to f7549a2 Compare September 20, 2023 07:14
@DCNick3 DCNick3 marked this pull request as ready for review September 21, 2023 07:56
@DCNick3 DCNick3 mentioned this pull request Sep 21, 2023
12 tasks
ilchu
ilchu previously approved these changes Sep 25, 2023
Copy link
Contributor

@ilchu ilchu left a comment

Choose a reason for hiding this comment

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

LGTM besides very minor points.

macro/utils/src/lib.rs Show resolved Hide resolved
data_model/derive/src/has_origin.rs Outdated Show resolved Hide resolved
data_model/derive/src/id.rs Outdated Show resolved Hide resolved
data_model/derive/src/id.rs Show resolved Hide resolved
macro/utils/src/lib.rs Show resolved Hide resolved
data_model/derive/src/filter.rs Outdated Show resolved Hide resolved
data_model/derive/src/filter.rs Outdated Show resolved Hide resolved
…in serde partially tagged enums

Signed-off-by: Nikita Strygin <[email protected]>
….filter_maps & get rid of unnecessary .except in derive(Filter)

This addresses the last of the concerns raised in hyperledger-iroha#2437

Signed-off-by: Nikita Strygin <[email protected]>
…cro, reduce repetition in derive(IdEqOrdHash), fix error reporting on stable

Signed-off-by: Nikita Strygin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants