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
Merged

Installer: supports windows #499

merged 28 commits into from
Jun 18, 2019

Conversation

axetroy
Copy link
Contributor

@axetroy axetroy 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

installer/mod.ts Show resolved Hide resolved
@axetroy axetroy changed the title Installer: supports windows [WIP] Installer: supports windows Jun 18, 2019
@axetroy axetroy changed the title [WIP] Installer: supports windows Installer: supports windows Jun 18, 2019
@axetroy
Copy link
Contributor Author

axetroy commented Jun 18, 2019

/cc @ry

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 Show resolved Hide resolved
@bartlomieju
Copy link
Member

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

@axetroy
Copy link
Contributor Author

axetroy 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
Copy link
Member

bartlomieju 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
Copy link
Contributor Author

axetroy commented Jun 18, 2019

@bardiarastin makes sense. I will finish it later.

@axetroy
Copy link
Contributor Author

axetroy 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?

installer/mod.ts Outdated Show resolved Hide resolved
installer/mod.ts Outdated Show resolved Hide resolved
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

Last nitpicks

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 Show resolved Hide resolved
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

LGTM

installer/mod.ts Outdated Show resolved Hide resolved
installer/mod.ts Outdated Show resolved Hide resolved
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

LGTM

@ry ry merged commit a68527f into denoland:master Jun 18, 2019
@axetroy axetroy deleted the installer_windows branch June 18, 2019 16:26
ry pushed a commit to ry/deno that referenced this pull request Oct 9, 2019
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.

None yet

4 participants