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

Add workspace-enabled example contracts #44

Closed
wants to merge 8 commits into from
Closed

Add workspace-enabled example contracts #44

wants to merge 8 commits into from

Conversation

aon
Copy link

@aon aon commented Oct 6, 2023

This PR creates a workspace where dependencies are defined and several smart-contracts as part of the same workspace, with inherited dependencies.

It was created following the recommendation made in use-ink/cargo-contract#1358. This should be merged after the mentioned PR is merged, given I had to force the branch version for running cargo-contract in the CI.

@ascjones
Copy link
Collaborator

ascjones commented Nov 7, 2023

use-ink/cargo-contract#1358 is merged now, please update this branch

@cla-bot-2021
Copy link

cla-bot-2021 bot commented Nov 8, 2023

User @tenuki, please sign the CLA here.

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.

Could we use some dumb contracts instead like contract1, contract2, contract3, with e.g. just a single message and constructor. We only need to demonstrate that they compile as part of a workspace.

It just means that when the original example e.g. erc20 changes we don't need to remember to update the copy of it here too.

}

#[cfg(test)]
mod tests {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we remove these and other tests? Reason is if the ink testing API changes then these will just be extra tests to update, they are just duplicates of existing ones anyway

Copy link
Contributor

Choose a reason for hiding this comment

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

Understood. I'll recheck / rework on this. Probably re-create the PR. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

(Also trying to adapt the merge to the actual HEAD of main version which changed the execution matrix)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll be creating a new PR to replace this one from a new temporary and updated repo, fixing this things. Sorry for the inconvenience. I'll update this PR with a link to the new-one.

@tenuki
Copy link
Contributor

tenuki commented Nov 14, 2023

This can be closed in favor of PR:52 : #52

@CoinFabrik CoinFabrik closed this by deleting the head repository Nov 15, 2023
ascjones pushed a commit that referenced this pull request Nov 21, 2023
* feature: check contracts in workspace

* fixes in CI

* fixes in CI

* fixes in CI: s/steps/step

* fixes in CI: enable examples step

* wip CI/CD: run contracts along regular ones, but with different step

* wip CI/CD: log changed files also use the right variable to check for them

* wip CI/CD: hopefuly preventing error on invalid character..

* wip CI/CD: changed approach to run both workspace contracts on same runner..

* wip CI/CD: whole workspace at once

* wip CI/CD: specify workspace cargo

* wip CI/CD: no test on workspace

* wip CI/CD: removed debug info/vars

* wip CI/CD: removed debug info/vars

* updated the CI/CD to be ready when using/having cargo-contract v 4.0.

* fix unnoticed error in contract

* Update ci.yml removed redundant configuration

Update ci.yml removed redundant configuration
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.

4 participants