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

refactor(cli): move snapshot_from_lockfile function to deno_npm #20024

Merged
merged 8 commits into from
Aug 8, 2023

Conversation

magurotuna
Copy link
Member

@magurotuna magurotuna commented Aug 2, 2023

This commit moves snapshot_from_lockfile function to deno_npm crate. This allows this function to be called outside Deno CLI (in particular, Deno Deploy).

Depends on:

@magurotuna
Copy link
Member Author

Seems like the test cases fail because the error messages are changed. I'll fix them

Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

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

Almost there.

Cargo.toml Outdated Show resolved Hide resolved
@@ -819,16 +820,26 @@ impl CliOptions {

if let Some(lockfile) = self.maybe_lockfile() {
if !lockfile.lock().overwrite {
return Ok(Some(
snapshot_from_lockfile(lockfile.clone(), api)
Copy link
Member

Choose a reason for hiding this comment

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

I think the code block below and the one in the language server are essentially the same thing? Could we maintain this snapshot_from_lockfile function in the CLI and then just have that call the deno_npm functions?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, how about this 9aa1e2d

@bartlomieju bartlomieju changed the title refactor(cli): move snapshot_from_locklfile function to deno_npm refactor(cli): move snapshot_from_lockfile function to deno_npm Aug 7, 2023
Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

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

LGTM!

@magurotuna magurotuna merged commit f2e30a6 into denoland:main Aug 8, 2023
13 checks passed
@magurotuna magurotuna deleted the snapshot_from_lockfile branch August 8, 2023 17:07
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

2 participants