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

Compile dylints when compiling the contract #703

Merged
merged 3 commits into from
Aug 24, 2022
Merged

Compile dylints when compiling the contract #703

merged 3 commits into from
Aug 24, 2022

Conversation

athei
Copy link
Contributor

@athei athei commented Aug 23, 2022

This removes the brittle setup of compiling the dylints within our build.rs file. Instead, it patches the contracts manifest to run the dylint. The dylint itself will live inside the ink! repository and is downloaded from there. The build is quick and cached within the target directory as all other artifacts.

Future Work

We don't want to pull the lint from master but rather from a tag that corresponds to the used ink! version. However, we don't wait for this because that is only possible once the next ink! version is released.

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.

🚀

src/workspace/manifest.rs Outdated Show resolved Hide resolved
src/cmd/build.rs Show resolved Hide resolved
@athei athei marked this pull request as ready for review August 24, 2022 09:12
@athei athei requested review from a team, cmichi and HCastano as code owners August 24, 2022 09:12
@athei athei merged commit 59247ac into master Aug 24, 2022
@athei athei deleted the at/dylint branch August 24, 2022 09:13
cmichi added a commit to paritytech/scripts that referenced this pull request Aug 24, 2022
cmichi added a commit to paritytech/scripts that referenced this pull request Aug 24, 2022
@ascjones ascjones mentioned this pull request Feb 14, 2023
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.

2 participants