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 nu-check #12137

Merged
merged 3 commits into from Mar 9, 2024
Merged

Refactor nu-check #12137

merged 3 commits into from Mar 9, 2024

Conversation

kubouch
Copy link
Contributor

@kubouch kubouch commented Mar 9, 2024

Description

This PR refactors nu-check and makes it possible to check module directories. Also removes the requirement for files to end with .nu: It was too limiting for module directories and there are executable scripts around that do not end with .nu, it's a common practice for scripts to omit it.

Other changes are:

  • Removed the --all flag and heuristic parse because these are irrelevant now when module syntax is a subset of script syntax (i.e., every module can be parsed as script).
  • Reduced code duplication and in general tidied up the code
  • Replaced unspanned errors with spanned ones.

User-Facing Changes

  • nu-check doesn't require files to end with .nu
  • can check module directories
  • Removed --all flag

Tests + Formatting

After Submitting

@kubouch kubouch added pr:breaking-change This PR implies a change affecting users and has to be noted in the release notes pr:commands This PR changes our commands in some way labels Mar 9, 2024
@fdncred
Copy link
Collaborator

fdncred commented Mar 9, 2024

Thanks for cleaning this up. People have been asking for this. IIRC, there are a few issues this will close.

@kubouch kubouch merged commit 5e937ca into nushell:main Mar 9, 2024
19 checks passed
@kubouch kubouch added this to the v0.92.0 milestone Mar 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:breaking-change This PR implies a change affecting users and has to be noted in the release notes pr:commands This PR changes our commands in some way
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants