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

Linter: Prepare the ink_linting crates for publishing #2060

Merged
merged 12 commits into from
Jan 15, 2024

Conversation

jubnzv
Copy link
Member

@jubnzv jubnzv commented Jan 12, 2024

Summary

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

Various improvements in ink_linting that allow to publish the its crates on crates.io.
This is necessary for use-ink/cargo-contract#1412.

Description

To publish ink_linting on crates.io, we need to prepare the Cargo workspace in linting and resolve all local dependencies to ensure they are also publishable.

The summary to the changes present here:

  1. Published the parity_clippy_utils crate on crates.io and added it as a dependency to ink_linting. We had to do this, because all the dependencies must be present on crates.io, but the original clippy_utils are not. For more context, see: Publishing `clippy_utils` on crates.io rust-lang/rust-clippy#12111 (comment) and Load lints from plugins rust-lang/rust-clippy#6746 (comment).
  2. Created the separate ink_linting_utils crate that basically contains the existing utilities functions shared between lints and wraps the parity_clippy_utils trait.
  3. Various changes in Cargo.toml files to make cargo publish happy.

The ink_linting and ink_linting_mandatory crates are currently reserved on crates.io and could be replaced with the real ones as soon as this PR is reviewed.

Checklist before requesting a review

  • My code follows the style guidelines of this project
  • I have added an entry to CHANGELOG.md
  • 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
  • Any dependent changes have been merged and published in downstream modules

We need this in order to publish `ink_linting` crates on crates.io,
because all the dependencies must be published as well.
This is necessary, because we need all the dependencies available on
crates.io to publish our crates.
To clarify the crate name when publishing on crates.io.
@codecov-commenter
Copy link

codecov-commenter commented Jan 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (af90adc) 53.62% compared to head (9a113ea) 53.58%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2060      +/-   ##
==========================================
- Coverage   53.62%   53.58%   -0.05%     
==========================================
  Files         224      224              
  Lines        7107     7107              
  Branches     3146     3146              
==========================================
- Hits         3811     3808       -3     
- Misses       3296     3299       +3     

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

@jubnzv jubnzv marked this pull request as ready for review January 12, 2024 17:54
Copy link

🦑 📈 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
basic-contract-caller 3.207 3.207 0 0
basic-contract-caller/other-contract 1.581 1.581 0 0
call-builder-return-value 8.904 8.904 0 0
call-runtime 2.013 2.013 0 0
conditional-compilation 1.453 1.453 0 0
contract-storage 7.336 7.336 0 0
contract-terminate 1.336 1.336 0 0
contract-transfer 1.693 1.693 0 0
custom-allocator 7.652 7.652 0 0
dns 7.321 7.321 0 0
e2e-call-runtime 1.302 1.302 0 0
e2e-runtime-only-backend 1.879 1.879 0 0
erc1155 14.123 14.123 0 0
erc20 6.918 6.918 0 0
erc721 9.816 9.816 0 0
events 5.264 5.264 0 0
flipper 1.637 1.637 0 0
incrementer 1.504 1.504 0 0
lang-err-integration-tests/call-builder-delegate 2.561 2.561 0 0
lang-err-integration-tests/call-builder 5.087 5.087 0 0
lang-err-integration-tests/constructors-return-value 1.987 1.987 0 0
lang-err-integration-tests/contract-ref 4.568 4.568 0 0
lang-err-integration-tests/integration-flipper 1.815 1.815 0 0
lazyvec-integration-test 4.553 4.553 0 0
mapping-integration-tests 7.919 7.919 0 0
mother 12.756 12.756 0 0
multi-contract-caller 6.155 6.155 0 0
multi-contract-caller/accumulator 1.378 1.378 0 0
multi-contract-caller/adder 1.908 1.908 0 0
multi-contract-caller/subber 1.928 1.928 0 0
multisig 21.621 21.621 0 0
payment-channel 5.653 5.653 0 0
sr25519-verification 1.148 1.148 0 0
static-buffer 1.649 1.649 0 0
trait-dyn-cross-contract-calls 2.706 2.706 0 0
trait-dyn-cross-contract-calls/contracts/incrementer 1.549 1.549 0 0
trait-erc20 7.294 7.294 0 0
trait-flipper 1.453 1.453 0 0
trait-incrementer 1.614 1.614 0 0
upgradeable-contracts/delegator 3.152 3.152 0 0
upgradeable-contracts/delegator/delegatee 1.613 1.613 0 0
upgradeable-contracts/set-code-hash 1.747 1.747 0 0
upgradeable-contracts/set-code-hash/updated-incrementer 1.726 1.726 0 0
wildcard-selector 2.852 2.852 0 0

Link to the run | Last update: Fri Jan 12 20:15:03 CET 2024

Copy link
Collaborator

@ascjones ascjones left a comment

Choose a reason for hiding this comment

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

LGTM

@jubnzv jubnzv merged commit 49db0b5 into use-ink:master Jan 15, 2024
23 checks passed
@SkymanOne SkymanOne mentioned this pull request Jan 15, 2024
@SkymanOne SkymanOne mentioned this pull request Feb 8, 2024
@SkymanOne SkymanOne mentioned this pull request Mar 4, 2024
@flip1995
Copy link

flip1995 commented Nov 28, 2024

FYI: clippy_utils is now being published to crates.io: rust-lang/rust-clippy#13556

https://crates.io/crates/clippy_utils

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants