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

POC: Add sprig library #1312

Merged
merged 1 commit into from
Jan 25, 2022
Merged

POC: Add sprig library #1312

merged 1 commit into from
Jan 25, 2022

Conversation

angrycub
Copy link
Contributor

@angrycub angrycub commented Nov 12, 2019

This PR adds the Sprig template functions library. For clarity and ease of documentation, the sprig functions are renamed sprig_«function».

@angrycub
Copy link
Contributor Author

angrycub commented Nov 12, 2019

Here is a template that I used to experiment with Sprig (list, append, and sprig's join renamed as sprig_join)

{{- $service := service "nomad" -}}
{{- scratch.Set "acc" sprig_list -}}
{{- range $service -}}
  {{- $curAcc := scratch.Get "acc" -}}
  {{- scratch.Set "acc" (sprig_append $curAcc (printf "%s:%v" .Address .Port) ) -}}
{{- end -}}
{{- scratch.Get "acc" | sprig_join "," }}

@eikenb
Copy link
Contributor

eikenb commented Apr 22, 2020

Hey @angrycub, thanks for taking the time to submit a PR.

I hadn't heard of the sprig library but this definitely looks like it could be useful. I'm not going to put this in the upcoming release (0.25.0) that I'm preparing for as this will take a bit of time to research and I'm not sure how to handle documentation for it (plus I really want to rework the documentation into a better format first).

@freeseacher
Copy link

Can't wait to see it being merged!

@eikenb
Copy link
Contributor

eikenb commented Aug 13, 2020

@freeseacher .. Thanks for the interest. Be sure to 👍 the PR (top post) to vote for this. We use the 👍's as a way to judge community priority. Thanks.

@vbatuev
Copy link

vbatuev commented Aug 13, 2020

It's very useful feature. Thank you!

@angrycub
Copy link
Contributor Author

angrycub commented Aug 13, 2020

@eikenb - I was looking at this as I was rebasing it last, and I was thinking. What about if we decorate all of the sprig functions so that you could say that we provide them as sprig_* , rather than merging them into the top level, and then provided a sprig_version function so that someone could determine what version of sprig was compiled into the library? That would help simplify the documentation.

@angrycub angrycub mentioned this pull request Nov 6, 2020
@eikenb
Copy link
Contributor

eikenb commented Apr 27, 2021

@angrycub... I'm back! I like the idea of prefixing the functions to make it an obvious namespace. If you're still interested, please update the PR with your idea. Thanks!

@eikenb
Copy link
Contributor

eikenb commented Apr 27, 2021

@angrycub, if you update this would you please consider adding at least a small/stub update to the documentation. Just a short something about the sprig project/library and a link to its documentation would be great. Thanks!

@eikenb
Copy link
Contributor

eikenb commented Apr 29, 2021

Note to self. I spoke with @angrycub via slack (didn't notice they were another hashi employee) and he wants to look into another way of organizing it (inspired by hugo's template setup) and will re-submit.

@hashicorp-cla
Copy link

hashicorp-cla commented Sep 9, 2021

CLA assistant check
All committers have signed the CLA.

@cucxabong
Copy link

any update on this

@eikenb
Copy link
Contributor

eikenb commented Sep 20, 2021

@angrycub .. were you still thinking about reworking your approach for this feature/PR?

@Chinikins
Copy link

is there an update on this, it would be super useful

@bitsofinfo
Copy link

would love this!

@eikenb
Copy link
Contributor

eikenb commented Jan 24, 2022

I'll add this to the 0.28.0 milestone and look at it for that release.

@eikenb eikenb added this to the v0.28.0 milestone Jan 24, 2022
@angrycub
Copy link
Contributor Author

I looked into namespacing the sprig funcs, and it's (kind of) possible, but you end up having to upcase the first letter of the sprig function name so that they are exported funcs on a returned object. It's not the ugliest thing I've ever seen, but it does push it a little further away from the documented names--and seems to provide no real bonus usability compared to just whacking sprig_ in front of the func names.

I can get this PR back into shape prepending sprig_ if you'd like, or we can have a quick convo about the namespacing attempts.

@angrycub
Copy link
Contributor Author

I updated the early version PR comments to reflect the current truth of the PR (prefixes and for all the funcs and the sample template using the funcs) as well

Copy link
Contributor

@eikenb eikenb left a comment

Choose a reason for hiding this comment

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

👍 LGTM

Thanks!

@eikenb eikenb added hashicat-update-required Changes that need to be ported to hashicat and removed waiting-reply labels Jan 24, 2022
@eikenb
Copy link
Contributor

eikenb commented Jan 24, 2022

[ninja edit: fixed referenced bug #]

Fixes #1206

Seeing if that will auto-close the ticket. Not sure if comments trigger that.

@angrycub angrycub merged commit 6ae6d3b into hashicorp:master Jan 25, 2022
@angrycub angrycub deleted the f-sprig branch January 25, 2022 00:27
@Chinikins
Copy link

thanks for the quick update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement hashicat-update-required Changes that need to be ported to hashicat
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants