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

bake: add indexof hcl func #2384

Merged
merged 2 commits into from
Apr 11, 2024
Merged

Conversation

Usual-Coder
Copy link
Contributor

@Usual-Coder Usual-Coder commented Apr 6, 2024

Fix #2383

@Usual-Coder Usual-Coder changed the title Fix #2383 bake: fix hcl index function Apr 6, 2024
Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

As we have already shipped a different functionality under name "index" (and index function to return element at specific index from array isn't very unusual) I don't think we can just change the definition. The new functionality looks ok but we should use a different name, maybe "indexof", or "search"?

hack/demo-env/examples/feature-hcl-index/test.hcl Outdated Show resolved Hide resolved
Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

We still need tests for this functionality as mentioned in previous comment. Let us know if you need any extra guidance with that.

// IndexFunc constructs a function that finds the element index for a given value in a list.
//
// This function was imported from terraform's collection functions.
var IndexFunc = function.New(&function.Spec{
Copy link
Member

Choose a reason for hiding this comment

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

rename to IndexOfFunc

Copy link
Contributor Author

@Usual-Coder Usual-Coder Apr 11, 2024

Choose a reason for hiding this comment

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

  • Renaming IndexOfFunc done
  • Guidance is more than welcome = as explained, I didn't do much and "copy/pasted" (from terraform) a similar solution @jedevc had done 2 years ago with timestampFunc = Can you point me where the corresponding tests are ... or any related to the set of functions mapped/created in this bake/hclparser/stdlib.go? Thanks

@crazy-max crazy-max added this to the v0.14.0 milestone Apr 11, 2024
@crazy-max crazy-max changed the title bake: fix hcl index function bake: add indexof hcl func Apr 11, 2024
@crazy-max
Copy link
Member

Added extra commits to test this new func.

@crazy-max crazy-max merged commit df7a3db into docker:master Apr 11, 2024
85 checks passed
@Usual-Coder
Copy link
Contributor Author

@crazy-max (and @tonistiigi) thanks 🙇

@Usual-Coder Usual-Coder deleted the feature-hcl-index branch April 11, 2024 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bake: hcl index function doesn't work as expected
4 participants