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

lang: stabilise templatestring func experiment #35224

Merged
merged 3 commits into from
May 28, 2024
Merged

lang: stabilise templatestring func experiment #35224

merged 3 commits into from
May 28, 2024

Conversation

kmoe
Copy link
Member

@kmoe kmoe commented May 22, 2024 •
edited
Loading

A templatestring function was introduced as a language experiment in #34968. Based on feedback the function seems to do the job, so this PR concludes the experiment by making the function available in all configurations.

Closes #30616, closes #26838

@kmoe kmoe force-pushed the kmoe/stabilise-templatestring-experiment branch from b78c593 to d04a9f2 Compare May 22, 2024 12:48
@kmoe kmoe force-pushed the kmoe/stabilise-templatestring-experiment branch from d04a9f2 to c137779 Compare May 22, 2024 14:54
apparentlymart
apparentlymart reviewed May 22, 2024
View reviewed changes
Copy link
Contributor

@apparentlymart apparentlymart left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This seems plausible to me so far!

(Sorry, I didn't notice this was a draft when I first looked at it, so this feedback may be jumping the gun.)

internal/command/jsonfunction/function.go Outdated Show resolved Hide resolved
internal/lang/functions_test.go
@@ -1311,9 +1327,14 @@ func TestFunctions(t *testing.T) {

for _, test := range funcTests {
t.Run(test.src, func(t *testing.T) {
data := &dataForTests{} // no variables available; we only need literals here
data := &dataForTests{
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess in some later PR we might want to factor this "fake evaluation data for tests" idea out into a single shared location, since I notice we now have at least two variations of it! (One in package lang, and another in package terraform.)

That's for another day, though. Not important for this PR.

@kmoe kmoe force-pushed the kmoe/stabilise-templatestring-experiment branch from c137779 to e0e1c1e Compare May 24, 2024 08:48
@kmoe kmoe force-pushed the kmoe/stabilise-templatestring-experiment branch from e0e1c1e to 1c39d1b Compare May 24, 2024 08:54
@kmoe kmoe force-pushed the kmoe/stabilise-templatestring-experiment branch from 1c39d1b to f25a95d Compare May 24, 2024 09:04
@kmoe kmoe force-pushed the kmoe/stabilise-templatestring-experiment branch from f25a95d to 93ba58f Compare May 24, 2024 10:27
@kmoe kmoe force-pushed the kmoe/stabilise-templatestring-experiment branch from 93ba58f to ec3641c Compare May 24, 2024 11:44
@kmoe kmoe marked this pull request as ready for review May 24, 2024 11:44
@kmoe kmoe requested a review from apparentlymart May 24, 2024 11:44
@kmoe kmoe merged commit 1e2d4a2 into main May 28, 2024
7 checks passed
@kmoe kmoe deleted the kmoe/stabilise-templatestring-experiment branch May 28, 2024 15:50
@github-actions GitHub Actions
Copy link
Contributor

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

@github-actions GitHub Actions
Copy link
Contributor

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@radeksimko radeksimko radeksimko left review comments

@jbardin jbardin jbardin approved these changes

@apparentlymart apparentlymart Awaiting requested review from apparentlymart

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@kmoe @apparentlymart @jbardin @radeksimko