Skip to content

Do not generate build artifacts under target for check#124

Merged
ascjones merged 7 commits intomasterfrom
cmichi-do-not-generate-target-artifcats-for-check
Jan 22, 2021
Merged

Do not generate build artifacts under target for check#124
ascjones merged 7 commits intomasterfrom
cmichi-do-not-generate-target-artifcats-for-check

Conversation

@cmichi
Copy link
Collaborator

@cmichi cmichi commented Dec 14, 2020

Our documentation for cargo contract check:

/// Check that the code builds as Wasm;
/// does not output any build artifact to the top level `target/` directory

I found that despite this comment we actually output to the target dir. This PR changes this behavior by making check build into a tmp dir.

@cmichi cmichi requested a review from ascjones December 14, 2020 13:48
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.

Rather than building into a tmp dir, did you consider running cargo check instead of cargo build when invoking cargo for wasm and metadata gen?

e.g. I just ran the raw command cargo check --target wasm32-unknown-unknown -Zbuild-std -Zbuild-std-features=panic_immediate_abort --no-default-features and it worked as expected.

@cmichi
Copy link
Collaborator Author

cmichi commented Jan 22, 2021

@ascjones Thanks for the comments, I applied all of them. Can you take another look please?

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

@ascjones ascjones merged commit 592b016 into master Jan 22, 2021
@ascjones ascjones deleted the cmichi-do-not-generate-target-artifcats-for-check branch January 22, 2021 22:44
@ascjones ascjones mentioned this pull request Feb 22, 2021
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