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: Add "deno check" subcommand for type checking #14072

Merged
merged 29 commits into from Apr 10, 2022

Conversation

bartlomieju
Copy link
Member

@bartlomieju bartlomieju commented Mar 22, 2022

Currently just an alias for "deno cache".

Prints warning for "deno run" subcommand if "--check" flag is not present
and there's no "--no-check" flag. Adds "DENO_FUTURE_CHECK" env
variable that allows to opt into new behavior now.

Closes #14071
Closes #11786

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

cli/main.rs Show resolved Hide resolved
cli/main.rs Show resolved Hide resolved
@ry
Copy link
Member

ry commented Mar 23, 2022

Please implement the env var described here #11786

@bartlomieju bartlomieju marked this pull request as draft March 26, 2022 22:46
cli/flags.rs Outdated Show resolved Hide resolved
cli/flags.rs Outdated Show resolved Hide resolved
cli/flags.rs Outdated Show resolved Hide resolved
cli/flags.rs Outdated Show resolved Hide resolved
cli/flags.rs Outdated Show resolved Hide resolved
cli/flags.rs Outdated Show resolved Hide resolved
cli/flags.rs Outdated Show resolved Hide resolved
cli/main.rs Show resolved Hide resolved
cli/main.rs Outdated Show resolved Hide resolved
@bartlomieju bartlomieju marked this pull request as ready for review March 30, 2022 14:52
cli/flags.rs Outdated
Comment on lines 727 to 730
Arg::new("all")
.long("all")
.help("Type-check all modules, including remote")
)
Copy link
Member Author

Choose a reason for hiding this comment

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

Rename to --remote?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Rename to --remote?

That might be better. If we made another category of diagnostics/locations opt-in, that should be its own flag e.g. deno check --remote --dts.

At this point I'm not sure if deno run --check is a good idea or if we should insist deno check is used to type check. If we kept it the API would have to be more like deno run --check=--remote,--dts.

Copy link
Member Author

Choose a reason for hiding this comment

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

At this point I'm not sure if deno run --check is a good idea

We need it for at least a few minor releases to give users time to migrate to deno check. I don't feel strongly about it, but I think we could punt this discussion until later time.

@bartlomieju bartlomieju requested review from ry and dsherret April 8, 2022 11:46
@bartlomieju
Copy link
Member Author

bartlomieju commented Apr 8, 2022

@ry this is now ready to land, merge on approval

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

@bartlomieju bartlomieju merged commit 8ae1702 into denoland:main Apr 10, 2022
@bartlomieju bartlomieju deleted the deno_check branch April 10, 2022 23:13
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.

deno check deno check
5 participants