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

collect supplements in issue contract #231

Closed

Conversation

cymqqqq
Copy link
Contributor

@cymqqqq cymqqqq commented Jun 27, 2024

Description:

add collect supplements feature for the issue_contract, includes the following Supplement instance:

genesis supplement
iface supplement
iimpl supplement
schema supplement

Copy link

codecov bot commented Jun 27, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 8 lines in your changes missing coverage. Please review.

Project coverage is 18.1%. Comparing base (7f637c6) to head (e599ba1).
Report is 1 commits behind head on master.

Files Patch % Lines
src/interface/builder.rs 50.0% 8 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #231     +/-   ##
========================================
+ Coverage    17.6%   18.1%   +0.6%     
========================================
  Files          37      37             
  Lines        7535    7548     +13     
========================================
+ Hits         1323    1369     +46     
+ Misses       6212    6179     -33     
Flag Coverage Δ
rust 18.1% <50.0%> (+0.6%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@dr-orlovsky dr-orlovsky left a comment

Choose a reason for hiding this comment

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

You can just do

supplements: confined_bset![
   Supplement::new(ContentRef::Schema(schema.schema_id()), schema.developer.clone()),
    ...
]

@dr-orlovsky dr-orlovsky added the enhancement New feature or request label Jun 27, 2024
@dr-orlovsky dr-orlovsky added this to the v0.11.0 milestone Jun 27, 2024
@dr-orlovsky
Copy link
Member

Why are you merging master? Just do rebases

@cymqqqq
Copy link
Contributor Author

cymqqqq commented Jun 27, 2024

Sorry....I always use the update branch button, next time I'll try to git rebase main.

Copy link
Member

@dr-orlovsky dr-orlovsky left a comment

Choose a reason for hiding this comment

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

Sorry, just understood that in this PR instead of gathering supplements from the stash you create some pseudo-supplements out of arbitrary cosnensus data.

Approach NACK. Please either collect supplements from the stash - or close the PR

@cymqqqq
Copy link
Contributor Author

cymqqqq commented Jun 28, 2024

I agree with you, the first idea is to collect supplements from the stash, but the question is: do we have a stash when we issue a contract? (In the consign method, yes, there is already a stash so we can collect supplements from the stash.) I just found ContractBuilder, and after we finish the contract issue, then initialize a stock(MemStash, MemState, and MemIndex), and then we can import the kit and contract to the stash.

@dr-orlovsky
Copy link
Member

When you issue contract you need to collect just supplements for interfaces, schema and implementation. There is no "genesis supplement" which can be found in the stash.

You also need to keep in mind that right now there is no supplements which exist - this functionality is there only as a way to add new things in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants