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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

replace let-env FOO = ... by $env.FOO = ... #972

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

amtoine
Copy link
Member

@amtoine amtoine commented Jul 1, 2023

related to nushell/nushell#9574

this PR moves from using let-env FOO = ... to using $env.FOO = ... in all the website.

procedure

this was an automatic commit, i've simply run the following command 馃憤

sd --string-mode "let-env " '$env.' book/**/*

cc/ @jntrnr

command used:
```nushell
sd --string-mode "let-env " '$env.' book/**/*
```
@fdncred
Copy link
Collaborator

fdncred commented Jul 1, 2023

I'm kind of on the fence about landing this now because it's such a huge breaking release and we haven't published a release yet that contains these changes. I could go either way.

@amtoine
Copy link
Member Author

amtoine commented Jul 2, 2023

I'm kind of on the fence about landing this now because it's such a huge breaking release and we haven't published a release yet that contains these changes. I could go either way.

it can wait for the release, at least the change is done here and can be used anytime 馃槍

on the other hand, changing from let-env FOO = ... to $env.FOO = ... on the website is not a breaking change right now because both syntaxes are supported, right?

@fdncred
Copy link
Collaborator

fdncred commented Jul 2, 2023

I'd call it a breaking change because anyone who previously used it will now be broken.

@amtoine
Copy link
Member Author

amtoine commented Jul 3, 2023

but now, let-env has been removed, so this is a breaking change for the greater good 馃枛 馃槈

@fdncred
Copy link
Collaborator

fdncred commented Jul 3, 2023

it's a breaking change for the same reaons this nushell/nushell#9574 is a breaking change

@amtoine
Copy link
Member Author

amtoine commented Jul 3, 2023

do you mean that if we change this now, people using the latest stable release will have a broken doc?

if so, yeah, we need to wait for the next release 馃憤

@amtoine amtoine added the wait-after-next-release The PR will be merged after next Nu release label Jul 3, 2023
@fdncred
Copy link
Collaborator

fdncred commented Jul 3, 2023

do you mean that if we change this now, people using the latest stable release will have a broken doc?

yes

@sophiajt sophiajt merged commit 0aa7b02 into nushell:main Jul 25, 2023
2 checks passed
@sophiajt
Copy link
Contributor

@fdncred they would not have broken docs because $env.FOO = ... has been supported for a few releases.

@fdncred
Copy link
Collaborator

fdncred commented Jul 25, 2023

@jntrnr removing let-env was the breaking change I was referring to.

@amtoine amtoine deleted the let-env-is-deprecated branch July 26, 2023 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wait-after-next-release The PR will be merged after next Nu release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants