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.

Sign up for GitHub

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

Jump to bottom

Remove lazy records #12682

Merged
merged 10 commits into from May 3, 2024
Merged

Remove lazy records #12682

merged 10 commits into from May 3, 2024

Conversation

IanManske
Copy link
Member

Description

Removes lazy records from the language, following from the reasons outlined in #12622. Namely, this should make semantics more clear and will eliminate concerns regarding maintainability.

User-Facing Changes

  • Breaking change: lazy make is removed.
  • Breaking change: describe --collect-lazyrecords flag is removed.
  • sys and debug info now return regular records.

After Submitting

  • Update nushell book if necessary.
  • Explore new sys and debug info APIs to prevent them from taking too long (e.g., subcommands or taking an optional column/cell-path argument).

@IanManske IanManske added wait-until-after-nushell-release pr:breaking-change This PR implies a change affecting users and has to be noted in the release notes pr:release-note-mention Addition/Improvement to be mentioned in the release notes pr:commands This PR changes our commands in some way pr:language This PR makes some language changes labels Apr 27, 2024
@IanManske IanManske linked an issue Apr 28, 2024 that may be closed by this pull request
Are lazy records worth it? #12622
Closed
@IanManske IanManske marked this pull request as ready for review May 2, 2024 03:02
WindSoilder
WindSoilder approved these changes May 3, 2024
View reviewed changes
Copy link
Collaborator

@WindSoilder WindSoilder left a comment

Choose a reason for hiding this comment

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

Thanks! I think it's ok to merge it

@WindSoilder WindSoilder merged commit 847646e into nushell:main May 3, 2024
15 checks passed
@hustcer hustcer added this to the v0.94.0 milestone May 3, 2024
@sholderbach
Copy link
Member

Do we have a ball-park assessment what that does to sys uses? Things that are possibly used in prompts or completions.

@IanManske IanManske added the removal-after-deprecation The component has already been sunset with `deprecation` and is now up for final removal label May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WindSoilder WindSoilder WindSoilder approved these changes

Assignees
No one assigned
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 pr:language This PR makes some language changes pr:release-note-mention Addition/Improvement to be mentioned in the release notes removal-after-deprecation The component has already been sunset with `deprecation` and is now up for final removal wait-until-after-nushell-release
Projects
None yet
Milestone
v0.94.0
Development

Successfully merging this pull request may close these issues.

Are lazy records worth it?
4 participants
@IanManske @sholderbach @WindSoilder @hustcer