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

Installer: supports windows #499

Merged
merged 28 commits into from Jun 18, 2019

Conversation

4 participants
@axetroy
Copy link
Contributor

commented Jun 18, 2019

ref #497

Refer to the implementation of npm

current supports run in CMD/PowerShell/Git Bash

try it out

deno run -A https://github.com/axetroy/deno_std/raw/installer_windows/installer/mod.ts file_server https://deno.land/std/http/file_server.ts --allow-net --allow-read

cmd

1

powershell

2

git bash

3

Show resolved Hide resolved installer/mod.ts

axetroy added some commits Jun 18, 2019

@axetroy axetroy changed the title Installer: supports windows [WIP] Installer: supports windows Jun 18, 2019

axetroy added some commits Jun 18, 2019

fix

@axetroy axetroy changed the title [WIP] Installer: supports windows Installer: supports windows Jun 18, 2019

@axetroy

This comment has been minimized.

Copy link
Contributor Author

commented Jun 18, 2019

/cc @ry

@axetroy axetroy referenced this pull request Jun 18, 2019

Closed

Installer: missing real test in different platforms #500

0 of 3 tasks complete

axetroy added some commits Jun 18, 2019

Show resolved Hide resolved installer/mod.ts
Show resolved Hide resolved installer/mod.ts Outdated
Show resolved Hide resolved installer/mod.ts Outdated
Show resolved Hide resolved installer/mod.ts
Show resolved Hide resolved installer/mod.ts
Show resolved Hide resolved installer/mod.ts

axetroy added some commits Jun 18, 2019

@bartlomieju

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2019

@axetroy can you replace code below // ensure script that is being installed exists with call to deno fetch?

@axetroy

This comment has been minimized.

Copy link
Contributor Author

commented Jun 18, 2019

@axetroy can you replace code below // ensure script that is being installed exists with call to deno fetch?

Can you explain why you need to use deno fetch? I will be appreciated

@bartlomieju

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2019

Sure: right now it downloads remote file using fetch or looks for local file using stat - this step is validating that file you're trying to install exists. It's fine, but a bit non-sense because downloaded file is discarded. We can just use deno fetch to do the same job (validating that file exists) as well as to download and compile deps so on first run it will just run skipping download/compilation step then.

@axetroy

This comment has been minimized.

Copy link
Contributor Author

commented Jun 18, 2019

@bardiarastin makes sense. I will finish it later.

@axetroy

This comment has been minimized.

Copy link
Contributor Author

commented Jun 18, 2019

@axetroy can you replace code below // ensure script that is being installed exists with call to deno fetch?

@bardiarastin I finished it, can you review it again?

Show resolved Hide resolved installer/mod.ts Outdated
Show resolved Hide resolved installer/mod.ts Outdated

axetroy added some commits Jun 18, 2019

@bartlomieju
Copy link
Contributor

left a comment

Last nitpicks

Show resolved Hide resolved installer/test.ts Outdated
Show resolved Hide resolved installer/mod.ts Outdated
Show resolved Hide resolved installer/mod.ts Outdated
Show resolved Hide resolved installer/mod.ts Outdated
Show resolved Hide resolved installer/mod.ts Outdated
Show resolved Hide resolved installer/mod.ts
Show resolved Hide resolved installer/mod.ts Outdated
Show resolved Hide resolved installer/mod.ts Outdated
Show resolved Hide resolved installer/mod.ts Outdated

axetroy added some commits Jun 18, 2019

@bartlomieju
Copy link
Contributor

left a comment

LGTM

Show resolved Hide resolved installer/mod.ts Outdated
Show resolved Hide resolved installer/mod.ts Outdated
@ry

ry approved these changes Jun 18, 2019

Copy link
Contributor

left a comment

LGTM

@ry ry merged commit a68527f into denoland:master Jun 18, 2019

5 checks passed

denoland.deno_std Build #20190618.25 succeeded
Details
denoland.deno_std (Linux) Linux succeeded
Details
denoland.deno_std (Mac) Mac succeeded
Details
denoland.deno_std (Windows) Windows succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@axetroy axetroy deleted the axetroy:installer_windows branch Jun 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.