Is it really correct, that it you use `val FOO by ...

# github-workflows-kt

[Vampire]

Is it really correct, that it you use val FOO by Contexts.env, you end up with just FOO instead of env.FOO?

[Piotr Krzemiński]

you seem to be right - I checked if there's a test for it. There is an end-to-end one, but with no assertion (!) and the printed output is empty instead of using the env vars' value: https://github.com/typesafegithub/github-workflows-kt/blame/8005a2e446b7c80cabb68ea9022b3fb2d87ef973/.github/workflows/end-to-end-tests.main.kts#L191 (+ the screenshot), and a unit test: https://github.com/typesafegithub/github-workflows-kt/blob/8005a2e446b7c80cabb68ea[…]github/typesafegithub/workflows/dsl/expressions/ContextsTest.kt

[Piotr Krzemiński]

please report a bug

[Piotr Krzemiński]

in fact, two bugs to be fixed: (1) consuming env vars + (2) no assertions in the end-to-end tests - it's not easy to add them always, but here we could add them fairly easily

[Vampire]

What I wondered is, that this was done intentionally in the lib.

[Vampire]

The propertyToExprPath at https://github.com/typesafegithub/github-workflows-kt/blob/main/github-workflows-k[…]b/typesafegithub/workflows/dsl/expressions/ExpressionContext.kt would do the right think I think.

[Vampire]

But it is overwritten to do the wrong thing here: https://github.com/typesafegithub/github-workflows-kt/blob/main/github-workflows-k[…]typesafegithub/workflows/dsl/expressions/contexts/EnvContext.kt

[Vampire]

Maybe @jmfayard remembers how he did that?

[Piotr Krzemiński]

shared some findings in the issue

[Piotr Krzemiński]

I'm wondering what's the best path forward here, to not break existing users before publishing a new major version. I'm thinking about adding something like Contexts.envContext that would actually support the env context and let us test support for it, and in v4 we'd:
• rename Contexts.envContext to Contexts.env
• rename Contexts.env to something else, ideally outside of Contexts, maybe Shell.env?

[Vampire]

I have no idea, honestly.
Whatever you do will cause headaches if you change the meaning of something existing.
Even if you now already add Shell.env and deprecate Contexts.env and then with v4 change the meaning of Contexts.env you will have users going from v3.4.0 to v4 and thus miss the deprecation message and just get changed behavior.

I don't think there is a really good way to do such a change unless you just consider the current behaviour a bug you simply fix. 😄

But either way, we should probably immediately introduce the replacement (Shell.env or whatever) and deprecated Contexts.env, so that users at least have a chance to migrate away from it proactively.

[Piotr Krzemiński]

when going to v4, the users will need to regenerate their YAMLs and hopefully, they will notice that something changed and got broken (broken = happy case, silently working = sad case)

[Vampire]

Maybe depends on how many other things change 😄

[Vampire]

And whether they actually look at the differences