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 all contracts if contract name not specified #121

Merged
merged 6 commits into from
Mar 13, 2023

Conversation

shunsukew
Copy link
Contributor

@shunsukew shunsukew commented Mar 1, 2023

Resolve #110

@shunsukew shunsukew marked this pull request as ready for review March 3, 2023 02:32
@shunsukew shunsukew requested a review from codespool March 3, 2023 02:32
packages/cli/src/commands/contract/compile.ts Show resolved Hide resolved
packages/cli/src/commands/contract/compile.ts Outdated Show resolved Hide resolved
packages/cli/src/commands/contract/compile.ts Outdated Show resolved Hide resolved
packages/cli/src/commands/contract/test.ts Outdated Show resolved Hide resolved
@shunsukew shunsukew requested a review from codespool March 8, 2023 05:16
Copy link
Collaborator

@codespool codespool left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@shunsukew shunsukew merged commit bb10178 into master Mar 13, 2023
@shunsukew shunsukew deleted the feature/compile-test-all-contracts branch March 13, 2023 03:35
this.error(`Cannot find a contract named ${args.contractName} in swanky.config.json`);
const contractNames = [];
if (flags.all) {
const contractList = readdirSync(path.resolve("contracts"), { withFileTypes: true });
Copy link
Contributor

@kziemianek kziemianek Mar 13, 2023

Choose a reason for hiding this comment

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

Aren't contracts listed in swanky.config.json ? Currently this logic leads to an error, for example in case of empty directiory.

Error: Cannot find contract info for test contract in swanky.config.json

I think we should treat swanky.config.json as a source of list of all contracts.

Copy link
Contributor

Choose a reason for hiding this comment

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

This would be as simple as

      for (const contractName in config.contracts) {
        contractNames.push(contractName);
      }

Copy link
Collaborator

Choose a reason for hiding this comment

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

You're right. We need to decide what will be the final source of truth. This idea of reading the contract directory and comparing to config is an artefact from the past I think.
We'll address that in the upcoming feature that will allow to init a project with an existing contract and sync the config file according to the folder contents #111

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's true... thank you so much! will fix it soon

shunsukew added a commit that referenced this pull request Mar 22, 2023
In this PR #121, wrongly
deleted `required` option for contract test command while resolving
merge conflicts.
shunsukew added a commit that referenced this pull request Mar 25, 2023
swanky-cli projects will have github actions test CI script by default.

This feature #121 needs
to be merged and released to work.
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.

contract compile should compile all contracts by default
3 participants