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

Feature run scripts #148

Closed
wants to merge 5 commits into from
Closed

Feature run scripts #148

wants to merge 5 commits into from

Conversation

shunsukew
Copy link
Contributor

@shunsukew shunsukew commented Apr 28, 2023

Resolve #144 & #134

What were changed

  • swanky script run [SCRIPT_NAME] added.
  • typedContracts folder moved from tests/typedContracts to typedContract at project root, so that both tests and scripts can use them.
  • flipper typedContracts are scaffolded when a project is initialized, so that a sample script 00_deploy.ts can use for showcasing how to deploy contracts by using typedContracts (usually they're generated by swanky contract compile where typechain-polkadot get executed).
  • typechain-polkadot >= 1.0.0-beta doesn't need artifacts such as .contract & metadata .json externally. They're embedded in contract-info folder generated by typechain-polkadot command. So, no need for having raw artifacts both in tests and scripts.

@shunsukew shunsukew marked this pull request as draft April 28, 2023 05:08
@shunsukew shunsukew linked an issue Apr 28, 2023 that may be closed by this pull request
@shunsukew shunsukew marked this pull request as ready for review May 1, 2023 10:41
@shunsukew
Copy link
Contributor Author

Same as swanky contract test, swanky script run also gets version duplication warnings.
This PR was for fixing dup warning except testing #142.

@shunsukew shunsukew requested a review from codespool May 1, 2023 10:43
@@ -52,6 +52,7 @@ export class TestContract extends Command {

const projectDir = path.resolve();
const testDir = path.resolve("test");
const typedContractsDir = path.resolve("typedCotracts")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo?

import { fork } from "child_process";
import path = require("node:path");

export class RunCommand extends BaseCommand<typeof RunCommand> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

having both script and run looks redundant.
Why not use just run, so it's more in line with convention in f.ex. npm (swanky run deploy)?

Comment on lines +34 to +39
await new Promise((resolve, reject) => {
const childProcess = fork(scriptPath, [], {
stdio: "inherit",
execArgv: ["--require", "ts-node/register/transpile-only"],
env: { ...process.env },
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have you considered dynamically loading and executing the script instead of running it in a separate process?
That way we could provide for example a runner class, and have it more standardised.
This has an example of it:
https://betterprogramming.pub/how-to-run-typescript-in-javascript-1545e8a36518

@@ -43,6 +43,15 @@ export async function copyTemplateFiles(
path.resolve(templatesPath, "github"),
path.resolve(projectPath, ".github")
);
await ensureDir(path.resolve(projectPath, "typedContracts"));
await copy(
path.resolve(templatesPath, "typedContracts", "flipper"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

what if the template is not flipper?

Copy link
Collaborator

Choose a reason for hiding this comment

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

if we are providing a framework, then all of this boilerplate should be abstracted in a runner class, and only f.ex. setConfig(), run() methods exposed to the user.

@shunsukew shunsukew changed the title Feature script framework Feature run scripts May 4, 2023
@shunsukew shunsukew marked this pull request as draft May 10, 2023 08:07
@shunsukew shunsukew closed this Jun 24, 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.

Managing deployment of complex set of wasm contracts TS scripts framework
2 participants