Conversation
Otherwise contracts could be built for e2e tests with the `std` feature.
All the tests are built locally for the host architecture, therefore the lint is not applied to them.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2001 +/- ##
==========================================
- Coverage 53.30% 53.27% -0.03%
==========================================
Files 220 220
Lines 6845 6845
Branches 0 3037 +3037
==========================================
- Hits 3649 3647 -2
- Misses 3196 3198 +2 ☔ View full report in Codecov by Sentry. |
🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑These are the results when building the
Link to the run | Last update: Tue Nov 28 13:24:01 CET 2023 |
ascjones
left a comment
ascjones
left a comment
There was a problem hiding this comment.
LGTM!
Next step then is to see if we can use this by default in cargo-contract and remove all the hacky code there.
|
@jubnzv please resolve the conflict and we can merge this |
|
@ascjones Thanks! I'd like to suggest a small fix here first. |
linting/src/no_main.rs
Outdated
| impl EarlyLintPass for NoMain { | ||
| fn check_crate(&mut self, cx: &EarlyContext<'_>, krate: &Crate) { | ||
| // Disable when building for e2e tests | ||
| if !is_contract_build(cx) { |
There was a problem hiding this comment.
Can we execute this lint on all the builds, even for e2e tests, making no_main mandatory for every ink! contract?
The point is that cargo-contract always runs linter for the host architecture. It is possible to hack it a bit, but should we? Why should we skip this lint for tests?
There was a problem hiding this comment.
E2E tests will still run the linter because they compile the contract code blob.
Maybe what we really mean/want is that when building the contract "natively" i.e. not no_std, which happens for unit tests and metadata generation. But those are usually executed on a different path so maybe we don't need to make the distinction here.
There was a problem hiding this comment.
Yes, I see. Let's try to introduce this lint for all contracts. I think it won't make any problems.
3 participants
Summary
Closes #1976
cargo-contractorpallet-contracts?Description
That PR adds the
no_mainlint.The implementation of the lint is straightforward, and it could be merged as it is.
However, its purpose is to enhance the
cargo-contractcompilation pipeline and it should run with everybuildcommand. Therefore,cargo-contractneeds to be patched to execute dylint-based lints, includingno_main, on each execution ofcargo contract build.Checklist before requesting a review
CHANGELOG.md