ink icon indicating copy to clipboard operation
ink copied to clipboard

Split examples into public and internal

Open smiasojed opened this issue 1 year ago • 5 comments

Summary

Closes #_

  • [ ] y/n | Does it introduce breaking changes?
  • [ ] y/n | Is it dependant on the specific version of cargo-contract or pallet-contracts?

Split examples located in integration-tests directory into public and internal

Description

Split examples to easily distinguish which are public and internal. Public examples will be synchronised with ink-examples repository

Checklist before requesting a review

  • [x] My code follows the style guidelines of this project
  • [ ] I have added an entry to CHANGELOG.md
  • [x] I have commented my code, particularly in hard-to-understand areas
  • [ ] I have added tests that prove my fix is effective or that my feature works
  • [x] Any dependent changes have been merged and published in downstream modules

smiasojed avatar Mar 01 '24 10:03 smiasojed

Could you describe the criteria to decide which contracts are public and which ones are internal?

DM's with Michi 🙈 .

Basically stuff that either was already in ink-examples or was added since 4.x with an educational benefit. It's quite a lot of examples now though. I think we're getting to a state where it's overwhelming. Maybe we should structure ink-examples better into subfolders.

cmichi avatar Mar 04 '24 13:03 cmichi

Could you describe the criteria to decide which contracts are public and which ones are internal?

Internal contracts for CI should be created solely for testing purposes and should not demonstrate any use cases or best practices that cannot be observed in other public contracts. In contrast, public contracts are those that provide value for users by demonstrating API usage with best practices.

These contracts, which were already in the ink-examples repo, I assumed are public. This is the theory, but it is not so black and white. For example, I am not sure about call-builder-return-value. I have added this as an example of call-builder API usage, but it is also used in other contracts. Now I think that I should exclude it from public contracts. What do you think?

smiasojed avatar Mar 04 '24 13:03 smiasojed

For example, I am not sure about call-builder-return-value. I have added this as an example of call-builder API usage, but it is also used in other contracts. Now I think that I should exclude it from public contracts. What do you think?

Looking at it again, I agree. I would remove it from the public examples.

cmichi avatar Mar 04 '24 15:03 cmichi

Internal contracts for CI should be created solely for testing purposes and should not demonstrate any use cases or best practices that cannot be observed in other public contracts. In contrast, public contracts are those that provide value for users by demonstrating API usage with best practices.

By that criteria we should include the events example in the public folder since that demonstrates using the new #[ink::event] syntax for events defined across multiple crates.

ascjones avatar Mar 07 '24 12:03 ascjones

🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑

These are the results when building the integration-tests/* contracts from this branch with cargo-contract and comparing them to ink! master:

Contract Upstream Size (kB) PR Size (kB) Diff (kB) Diff (%) Change
call-builder-return-value 9.249 9.249 0 0 :heavy_minus_sign:
e2e-runtime-only-backend 1.901 1.901 0 0 :heavy_minus_sign:
mother 12.753 12.753 0 0 :heavy_minus_sign:
sr25519-verification 1.154 1.154 0 0 :heavy_minus_sign:
call-runtime 2.071 2.071 0 0 :heavy_minus_sign:
combined-extension 2.132 2.149 0.017 0.797373 :chart_with_upwards_trend:
conditional-compilation 1.502 1.502 0 0 :heavy_minus_sign:
contract-storage 7.58 7.58 0 0 :heavy_minus_sign:
contract-terminate 1.369 1.369 0 0 :heavy_minus_sign:
contract-transfer 1.731 1.731 0 0 :heavy_minus_sign:
cross-contract-calls 7.732 7.732 0 0 :heavy_minus_sign:
cross-contract-calls/other-contract 1.595 1.595 0 0 :heavy_minus_sign:
custom-allocator 7.787 7.787 0 0 :heavy_minus_sign:
custom-environment 2.158 2.158 0 0 :heavy_minus_sign:
dns 7.355 7.355 0 0 :heavy_minus_sign:
e2e-call-runtime 1.32 1.32 0 0 :heavy_minus_sign:
erc1155 14.345 14.345 0 0 :heavy_minus_sign:
erc20 6.955 6.955 0 0 :heavy_minus_sign:
erc721 10.044 10.044 0 0 :heavy_minus_sign:
events 5.27 5.27 0 0 :heavy_minus_sign:
flipper 1.651 1.651 0 0 :heavy_minus_sign:
incrementer 1.516 1.516 0 0 :heavy_minus_sign:
multi-contract-caller 6.654 6.654 0 0 :heavy_minus_sign:
multi-contract-caller/accumulator 1.388 1.388 0 0 :heavy_minus_sign:
multi-contract-caller/adder 1.922 1.922 0 0 :heavy_minus_sign:
multi-contract-caller/subber 1.942 1.942 0 0 :heavy_minus_sign:
multisig 21.871 21.871 0 0 :heavy_minus_sign:
payment-channel 5.742 5.742 0 0 :heavy_minus_sign:
psp22-extension 7.083 7.083 0 0 :heavy_minus_sign:
rand-extension 2.977 2.977 0 0 :heavy_minus_sign:
static-buffer 2.578 2.578 0 0 :heavy_minus_sign:
trait-dyn-cross-contract-calls 2.899 2.899 0 0 :heavy_minus_sign:
trait-dyn-cross-contract-calls/contracts/incrementer 1.557 1.557 0 0 :heavy_minus_sign:
trait-erc20 7.331 7.331 0 0 :heavy_minus_sign:
trait-flipper 1.502 1.502 0 0 :heavy_minus_sign:
trait-incrementer 1.626 1.626 0 0 :heavy_minus_sign:
upgradeable-contracts/delegator 3.96 3.96 0 0 :heavy_minus_sign:
upgradeable-contracts/delegator/delegatee 1.641 1.641 0 0 :heavy_minus_sign:
upgradeable-contracts/delegator/delegatee2 1.641 1.641 0 0 :heavy_minus_sign:
upgradeable-contracts/set-code-hash-migration 1.755 1.755 0 0 :heavy_minus_sign:
upgradeable-contracts/set-code-hash-migration/migration 1.462 1.462 0 0 :heavy_minus_sign:
upgradeable-contracts/set-code-hash-migration/updated-incrementer 1.909 1.909 0 0 :heavy_minus_sign:
upgradeable-contracts/set-code-hash 1.755 1.755 0 0 :heavy_minus_sign:
upgradeable-contracts/set-code-hash/updated-incrementer 1.733 1.733 0 0 :heavy_minus_sign:
wildcard-selector 2.858 2.858 0 0 :heavy_minus_sign:

Link to the run | Last update: Wed Apr 3 10:17:52 CEST 2024

github-actions[bot] avatar Apr 02 '24 18:04 github-actions[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 61.28%. Comparing base (6451bb8) to head (7186459). Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2131      +/-   ##
==========================================
- Coverage   61.34%   61.28%   -0.06%     
==========================================
  Files         139      139              
  Lines        5709     5709              
  Branches     2421     2421              
==========================================
- Hits         3502     3499       -3     
- Misses       2207     2210       +3     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Apr 02 '24 18:04 codecov-commenter