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

Feat/deploy #82

Merged
merged 13 commits into from
Apr 29, 2024
Merged

Feat/deploy #82

merged 13 commits into from
Apr 29, 2024

Conversation

ittechhunter
Copy link
Collaborator

@ittechhunter ittechhunter commented Apr 19, 2024

Draft #77

console.log("not yet supported");
.description("Deploy the project")
.argument("[string]", "Workspace app name to deploy")
.option("--deploy-account-id <deployAccountId>", "Account under which component code should be deployed")
Copy link
Contributor

Choose a reason for hiding this comment

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

This all looks good, can you run bw help and update the README with this new command?

global.log = unmockedLog;
})

it('should build correctly without logs', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I like how you've set up these tests so far -- for test cases, I'm thinking:

  • should match expected input for bos-cli-rs (src/widget -> src), isolate that function in code call it "translate"
  • should exec with correct command -> compare the the exec method call after running deployApp

lib/deploy.ts Outdated

const config = await readConfig(path.join(src, "bos.config.json"), opts.network);

// Move files from "src/widget" to "src/"
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks great, how about moving L25-53 to a "translate" or "translateForBosCli" function so we can test it?

@ittechhunter
Copy link
Collaborator Author

Resolve #77

@@ -47,9 +47,9 @@ const app_example_1_output = {
"/build/ipfs.json": JSON.stringify({
"logo.svg": "QmHash",
}, null, 2) + "\n",
"/build/src/widget/hello.utils.module.js": "const hello = (name) => `Hello, ${name}!`;\nreturn { hello };\n",
"/build/src/widget/hello/utils.module.js": "const hello = (name) => `Hello, ${name}!`;\nreturn { hello };\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be a dot, not /

"/build/src/widget/index.jsx": "const hello = \"hi\";\nreturn hello(props);\n",
"/build/src/widget/nested.index.jsx": "const hello = \"hi\";\nreturn hello(props);\n",
"/build/src/widget/nested/index.jsx": "const hello = \"hi\";\nreturn hello(props);\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Here too

@@ -118,7 +118,7 @@ const app_example_2_output = {
"/build/ipfs.json": JSON.stringify({
"logo.svg": "QmHash",
}, null, 2) + "\n",
"/build/src/widget/hello.utils.module.js": "const hello = (name) => `Hello, ${name}!`;\nreturn { hello };\n",
"/build/src/widget/hello/utils.module.js": "const hello = (name) => `Hello, ${name}!`;\nreturn { hello };\n",
"/build/src/widget/index.jsx": "const hello = \"hi\";\nreturn hello(props);\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Here

tests/unit/deploy.ts Show resolved Hide resolved
@elliotBraem elliotBraem marked this pull request as ready for review April 26, 2024 14:24
@elliotBraem
Copy link
Contributor

From testing it out locally

deploy single

  1. checkout branch, yarn install
  2. yarn build
  3. cd examples/single
  4. ../../bin/bw.js deploy

Although I did not provide any signing details, I still see a log for "App deployed successfully". I would expect a message that says "Necessary values not provided, please provide private & public key for deploy"

Screenshot 2024-04-27 at 11 29 33 PM

Otherwise works well

deploy multi

  1. cd examples/multi
  2. ../../bin/bw.js deploy

Although I am in a multi and I didn't provide an app name, it still attempts to deploy; it stalls on "deploying app"; we should stop it if in a workspace (has bos.workspace.json) and no app name provided

../../bin/bw.js deploy HelloWorld works well!

@elliotBraem elliotBraem merged commit 214361a into NEARBuilders:main Apr 29, 2024
1 check passed
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